svn commit: r348338 - head/sys/fs/pseudofs

[John Baldwin]

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