Re: [PATCH 3/3] vfs: shave a branch in getname_flags

From: Jan Kara
Date: Wed Jun 05 2024 - 11:49:40 EST


On Tue 04-06-24 17:52:57, Mateusz Guzik wrote:
> Check for an error while copying and no path in one branch.
>
> Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx>

Looks good to me modulo the need for rechecking Christian already pointed
out. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>


Honza

> ---
> fs/namei.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 950ad6bdd9fe..f25dcb9077dd 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -148,9 +148,20 @@ getname_flags(const char __user *filename, int flags)
> result->name = kname;
>
> len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
> - if (unlikely(len < 0)) {
> - __putname(result);
> - return ERR_PTR(len);
> + /*
> + * Handle both empty path and copy failure in one go.
> + */
> + if (unlikely(len <= 0)) {
> + if (unlikely(len < 0)) {
> + __putname(result);
> + return ERR_PTR(len);
> + }
> +
> + /* The empty path is special. */
> + if (!(flags & LOOKUP_EMPTY)) {
> + __putname(result);
> + return ERR_PTR(-ENOENT);
> + }
> }
>
> /*
> @@ -188,14 +199,6 @@ getname_flags(const char __user *filename, int flags)
> }
>
> atomic_set(&result->refcnt, 1);
> - /* The empty path is special. */
> - if (unlikely(!len)) {
> - if (!(flags & LOOKUP_EMPTY)) {
> - putname(result);
> - return ERR_PTR(-ENOENT);
> - }
> - }
> -
> result->uptr = filename;
> result->aname = NULL;
> audit_getname(result);
> --
> 2.39.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR