Re: [PATCH] kobject: Fix slab-out-of-bounds in fill_kobj_path()

From: Greg KH <gregkh@linuxfoundation.org>
To: "wanghai (M)" <wanghai38@huawei.com>
Cc: Alice Chao <alice.chao@mediatek.com>,
	rafael@kernel.org, jesse.brandeburg@intel.com,
	anthony.l.nguyen@intel.com, intel-wired-lan@lists.osuosl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kobject: Fix slab-out-of-bounds in fill_kobj_path()
Date: Mon, 19 Dec 2022 16:39:05 +0100	[thread overview]
Message-ID: <Y6CFma6PPcvJDGje@kroah.com> (raw)
In-Reply-To: <dd4d0619-8287-1650-aa43-5db2e86ff8cd@huawei.com>

On Mon, Dec 19, 2022 at 11:27:58PM +0800, wanghai (M) wrote:
> Can I send v2 like this?
> 
> diff --git a/lib/kobject.c b/lib/kobject.c
> index a0b2dbfcfa23..3f97d903266a 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -112,7 +112,7 @@ static int get_kobj_path_length(struct kobject *kobj)
>         return length;
>  }
> 
> -static void fill_kobj_path(struct kobject *kobj, char *path, int length)
> +static int fill_kobj_path(struct kobject *kobj, char *path, int length)
>  {
>         struct kobject *parent;
> 
> @@ -121,12 +121,16 @@ static void fill_kobj_path(struct kobject *kobj, char
> *path, int length)
>                 int cur = strlen(kobject_name(parent));
>                 /* back up enough to print this name with '/' */
>                 length -= cur;
> +               if (length <= 0)
> +                       return -EINVAL;
>                 memcpy(path + length, kobject_name(parent), cur);
>                 *(path + --length) = '/';
>         }
> 
>         pr_debug("kobject: '%s' (%p): %s: path = '%s'\n",
> kobject_name(kobj),
>                  kobj, __func__, path);
> +
> +       return 0;
>  }
> 
>  /**
> @@ -141,13 +145,17 @@ char *kobject_get_path(struct kobject *kobj, gfp_t
> gfp_mask)
>         char *path;
>         int len;
> 
> +retry:
>         len = get_kobj_path_length(kobj);
>         if (len == 0)
>                 return NULL;
>         path = kzalloc(len, gfp_mask);
>         if (!path)
>                 return NULL;
> -       fill_kobj_path(kobj, path, len);
> +       if (fill_kobj_path(kobj, path, len)) {
> +               kfree(path);
> +               goto retry;
> +       }

Much nicer, thanks!