Re: [PATCH v7 5/6] xfs: sanity check inode mode when creating new dentry

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, Jan 11, 2017 at 07:08:57AM +0200, Amir Goldstein wrote:
> On Wed, Jan 11, 2017 at 1:15 AM, Darrick J. Wong
> <darrick.wong@oracle.com> wrote:
> > On Tue, Jan 10, 2017 at 05:39:16PM +0200, Amir Goldstein wrote:
> >> The helper xfs_dentry_to_name() is used by 2 different
> >> classes of callers: Callers that pass zero mode and don't care
> >> about the returned name.type field and Callers that pass
> >> non zero mode and do care about the name.type field.
> >>
> >> Change xfs_dentry_to_name() to not take the mode argument and
> >> change the call sites of the first class to not pass the mode
> >> argument.
> >>
> >> Create a new helper xfs_dentry_mode_to_name() which does pass
> >> the mode argument and returns -EFSCORRUPTED if mode is invalid.
> >> Callers that translate non zero mode to on-disk file type now
> >> check the return value and will export the error to user instead
> >> of staging an invalid file type to be written to directory entry.
> >>
> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >> ---
> ...
> >>
> >>       mode = S_IFLNK |
> >>               (irix_symlink_mode ? 0777 & ~current_umask() : S_IRWXUGO);
> >> -     xfs_dentry_to_name(&name, dentry, mode);
> >> +     error = xfs_dentry_mode_to_name(&name, dentry, mode);
> >> +     ASSERT(error == 0);
> >
> > Why not the usual 'if (error) goto out;' here?
> >
> 
> Because mode here is not coming from disk, nor from user input,
> so an error from converting S_IFLNK would be a programming bug.
> We could also ignore the return value in this case, let me know if you
> prefer that.

Nah, it's just eyebrow-raising since it's a deviation from the usual
paradigm.

> BTW, process question. Do you need me to re-send patched with
> your Reviewed-by, or will you add those when applying?

In general I plan to pull the most recent patch mail and add whatever
tags come in as replies, so if you resend a patch please add the tags
yourself.

--D

> 
> Thanks,
> Amir.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html