Date: Wed, 29 May 2019 09:06:54 -0700 From: John Baldwin <jhb@FreeBSD.org> To: Johannes Lundberg <johalun@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r348338 - head/sys/fs/pseudofs Message-ID: <746b1976-2627-ad0d-e4b2-4607d944f87d@FreeBSD.org> In-Reply-To: <201905282054.x4SKsxZ2083779@repo.freebsd.org> References: <201905282054.x4SKsxZ2083779@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5/28/19 1:54 PM, Johannes Lundberg wrote: > Author: johalun > Date: Tue May 28 20:54:59 2019 > New Revision: 348338 > URL: https://svnweb.freebsd.org/changeset/base/348338 > > Log: > pseudofs: Ignore unsupported commands in vop_setattr. > > Users of pseudofs (e.g. lindebugfs), should be able to receive > input from command line via commands like "echo 1 > /path/to/file". > Currently this fails because sh tries to truncate the file first and > vop_setattr returns not supported error for this. This patch simply > ignores the error and returns 0 instead. > > Reviewed by: imp (mentor), asomers > Approved by: imp (mentor), asomers > MFC after: 1 week > Differential Revision: D20451 FYI, this is supposed to be the URL which makes it easier to click in many e-mail clients. It seems a recent change in phab that it honors just the 'Dxxxx' tag as previously the URL was required for auto-close. (I sure wish the phab team would communicate when they change things like this.) > Modified: > head/sys/fs/pseudofs/pseudofs_vnops.c > > Modified: head/sys/fs/pseudofs/pseudofs_vnops.c > ============================================================================== > --- head/sys/fs/pseudofs/pseudofs_vnops.c Tue May 28 20:44:23 2019 (r348337) > +++ head/sys/fs/pseudofs/pseudofs_vnops.c Tue May 28 20:54:59 2019 (r348338) > @@ -967,7 +967,8 @@ pfs_setattr(struct vop_setattr_args *va) > PFS_TRACE(("%s", pn->pn_name)); > pfs_assert_not_owned(pn); > > - PFS_RETURN (EOPNOTSUPP); > + /* Silently ignore unchangeable attributes. */ > + PFS_RETURN (0); Did you consider only whitelisting setattr calls that set the size? This allows things like chown/chmod that won't actually work which might be confusing to users. You can see examples of how to do this in, e.g. ufs_setattr. The interface for VOP_SETATTR is that the various fields in vattr are set to VNOVAL by via VATTR_NULL and then the caller sets the fields they want to change. You can then reject attempts to set attributes you don't support. (e.g. see how ufs_setattr fails with EINVAL if fields it doesn't support aren't set to VNOVAL). You should also likely validate the passed in size and only permit a size of 0 for [f]truncate(). -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?746b1976-2627-ad0d-e4b2-4607d944f87d>