2022-04-25 23:35:52

by Thiago Becker

[permalink] [raw]
Subject: [PATCH] nfsrahead: fix oops caused by non-starndard naming schemes

brtfs uses a non standard naming scheme for its fs structures, which
causes nfsrahead to take a long time scanning all the memory
available and then crashes. This causes the udev to take forever to
quiesce and delays the system startup.

This t=patch refactors the way the device number is obtained to handle
this situation.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2078147
Signed-off-by: Thiago Becker <[email protected]>
---
tools/nfsrahead/main.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/tools/nfsrahead/main.c b/tools/nfsrahead/main.c
index b3af3aa8..83a389f7 100644
--- a/tools/nfsrahead/main.c
+++ b/tools/nfsrahead/main.c
@@ -26,27 +26,31 @@ struct device_info {
};

/* Convert a string in the format n:m to a device number */
-static dev_t dev_from_arg(const char *device_number)
+static int fill_device_number(struct device_info *info)
{
- char *s = strdup(device_number), *p;
+ char *s = strdup(info->device_number), *p;
char *maj_s, *min_s;
unsigned int maj, min;
- dev_t dev;
+ int err = -EINVAL;

maj_s = p = s;
- for ( ; *p != ':'; p++)
+ for ( ; *p != ':' && *p != '\0'; p++)
;

+ if (*p == '\0')
+ goto out_free;
+
+ err = 0;
*p = '\0';
min_s = p + 1;

maj = strtol(maj_s, NULL, 10);
min = strtol(min_s, NULL, 10);

- dev = makedev(maj, min);
-
+ info->dev = makedev(maj, min);
+out_free:
free(s);
- return dev;
+ return err;
}

#define sfree(ptr) if (ptr) free(ptr)
@@ -55,7 +59,7 @@ static dev_t dev_from_arg(const char *device_number)
static void init_device_info(struct device_info *di, const char *device_number)
{
di->device_number = strdup(device_number);
- di->dev = dev_from_arg(device_number);
+ di->dev = 0;
di->mountpoint = NULL;
di->fstype = NULL;
}
@@ -76,6 +80,8 @@ static int get_mountinfo(const char *device_number, struct device_info *device_i
char *target;

init_device_info(device_info, device_number);
+ if ((ret = fill_device_number(device_info)) < 0)
+ goto out_free_device_info;

mnttbl = mnt_new_table();

@@ -101,6 +107,7 @@ out_free_fs:
mnt_free_fs(fs);
out_free_tbl:
mnt_free_table(mnttbl);
+out_free_device_info:
free(device_info->device_number);
device_info->device_number = NULL;
return ret;
--
2.35.1


2022-05-02 18:32:47

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] nfsrahead: fix oops caused by non-starndard naming schemes



On 4/25/22 3:59 PM, Thiago Becker wrote:
> brtfs uses a non standard naming scheme for its fs structures, which
> causes nfsrahead to take a long time scanning all the memory
> available and then crashes. This causes the udev to take forever to
> quiesce and delays the system startup.
>
> This t=patch refactors the way the device number is obtained to handle
> this situation.
>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2078147
> Signed-off-by: Thiago Becker <[email protected]>
Committed... Thanks!

steved.

> ---
> tools/nfsrahead/main.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/tools/nfsrahead/main.c b/tools/nfsrahead/main.c
> index b3af3aa8..83a389f7 100644
> --- a/tools/nfsrahead/main.c
> +++ b/tools/nfsrahead/main.c
> @@ -26,27 +26,31 @@ struct device_info {
> };
>
> /* Convert a string in the format n:m to a device number */
> -static dev_t dev_from_arg(const char *device_number)
> +static int fill_device_number(struct device_info *info)
> {
> - char *s = strdup(device_number), *p;
> + char *s = strdup(info->device_number), *p;
> char *maj_s, *min_s;
> unsigned int maj, min;
> - dev_t dev;
> + int err = -EINVAL;
>
> maj_s = p = s;
> - for ( ; *p != ':'; p++)
> + for ( ; *p != ':' && *p != '\0'; p++)
> ;
>
> + if (*p == '\0')
> + goto out_free;
> +
> + err = 0;
> *p = '\0';
> min_s = p + 1;
>
> maj = strtol(maj_s, NULL, 10);
> min = strtol(min_s, NULL, 10);
>
> - dev = makedev(maj, min);
> -
> + info->dev = makedev(maj, min);
> +out_free:
> free(s);
> - return dev;
> + return err;
> }
>
> #define sfree(ptr) if (ptr) free(ptr)
> @@ -55,7 +59,7 @@ static dev_t dev_from_arg(const char *device_number)
> static void init_device_info(struct device_info *di, const char *device_number)
> {
> di->device_number = strdup(device_number);
> - di->dev = dev_from_arg(device_number);
> + di->dev = 0;
> di->mountpoint = NULL;
> di->fstype = NULL;
> }
> @@ -76,6 +80,8 @@ static int get_mountinfo(const char *device_number, struct device_info *device_i
> char *target;
>
> init_device_info(device_info, device_number);
> + if ((ret = fill_device_number(device_info)) < 0)
> + goto out_free_device_info;
>
> mnttbl = mnt_new_table();
>
> @@ -101,6 +107,7 @@ out_free_fs:
> mnt_free_fs(fs);
> out_free_tbl:
> mnt_free_table(mnttbl);
> +out_free_device_info:
> free(device_info->device_number);
> device_info->device_number = NULL;
> return ret;