On Tue, 28 May 2024 10:29:03 +0800
Xiao Ni <xni@xxxxxxxxxx> wrote:
> It reports buffer overflow detected when creating raid with big
> nvme devices. In my test, the size of the nvme device is 1.5T.
> It can't reproduce this with nvme device which size is smaller
> than 1T.
Hi Xiao,
Size of disks should have nothing to do with this. We are just parsing sysfs.
Weird..
>
> In function get_nvme_multipath_dev_hw_path it allocs memory in a for
> loop and the size it allocs is big. So if the iteration number is
> large, it has a risk that the stack space is larger than the limit.
> So move the memory allocation at the biginning of the funtion.
I would expect that memory is deallocated after each loop but the fix
is correct and I'm willing to take this because obviously it is a fix for
something.
I don't understand the problem but I trust you. Maybe varied size stack array is
a problem?
Probably, enough would be to just replace [strlen(dev_path) +
strlen(ent->d_name) + 1] by [PATH_MAX] but I'm quite confused why it is an
issue at all.
LGTM. Please fix typos raised by Paul and I will merge it.
Thanks,
Mariusz
>
> Fixes: d835518b6b53 ('imsm: nvme multipath support')
> Reported-by: Guang Wu <guazhang@xxxxxxxxxx>
> Signed-off-by: Xiao Ni <xni@xxxxxxxxxx>
> ---
> platform-intel.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/platform-intel.c b/platform-intel.c
> index 15a9fa5a..0732af2b 100644
> --- a/platform-intel.c
> +++ b/platform-intel.c
> @@ -898,6 +898,7 @@ char *get_nvme_multipath_dev_hw_path(const char *dev_path)
> DIR *dir;
> struct dirent *ent;
> char *rp = NULL;
> + char buf[PATH_MAX];
>
> if (strncmp(dev_path, NVME_SUBSYS_PATH, strlen(NVME_SUBSYS_PATH)) !=
> 0) return NULL;
> @@ -907,14 +908,13 @@ char *get_nvme_multipath_dev_hw_path(const char
> *dev_path) return NULL;
>
> for (ent = readdir(dir); ent; ent = readdir(dir)) {
> - char buf[strlen(dev_path) + strlen(ent->d_name) + 1];
>
> /* Check if dir is a controller, ignore namespaces*/
> if (!(strncmp(ent->d_name, "nvme", 4) == 0) ||
> (strrchr(ent->d_name, 'n') != &ent->d_name[0]))
> continue;
>
> - sprintf(buf, "%s/%s", dev_path, ent->d_name);
> + snprintf(buf, PATH_MAX, "%s/%s", dev_path, ent->d_name);
> rp = realpath(buf, NULL);
> break;
> }
