| Message ID | 1593643176-6206-2-git-send-email-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | misc patches | expand |
On Wed, 2020-07-01 at 17:39 -0500, Benjamin Marzinski wrote: > dev_loss_tmo is a u32 value. However the kernel sysfs code prints it > as > a signed integer. This means that if dev_loss_tmo is above INT_MAX, > the > sysfs value will be a negative number. Parsing this was causing > sysfs_set_rport_tmo() to fail. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Thanks for catching this. I think it can be fixed simpler, because strtoul() parses negative values just fine. See below. Regards, Martin >From 16eca9b0f340a13fee0c28ae52dffa578193f015 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski <bmarzins@redhat.com> Date: Wed, 1 Jul 2020 17:39:33 -0500 Subject: [PATCH] libmultipath: fix sysfs dev_loss_tmo parsing dev_loss_tmo is a u32 value. However the kernel sysfs code prints it as a signed integer. This means that if dev_loss_tmo is above INT_MAX, the sysfs value will be a negative number. Parsing this was causing sysfs_set_rport_tmo() to fail. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Signed-off-by: Martin Wilck <mwilck@suse.com> --- libmultipath/discovery.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c index 1d542ea..3c9803a 100644 --- a/libmultipath/discovery.c +++ b/libmultipath/discovery.c @@ -581,7 +581,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp) struct udev_device *rport_dev = NULL; char value[16], *eptr; char rport_id[32]; - unsigned long long tmo = 0; + unsigned int tmo; int ret; sprintf(rport_id, "rport-%d:%d-%d", @@ -605,8 +605,8 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp) "error %d", rport_id, -ret); goto out; } - tmo = strtoull(value, &eptr, 0); - if (value == eptr || tmo == ULLONG_MAX) { + tmo = strtoul(value, &eptr, 0); + if (value == eptr) { condlog(0, "%s: Cannot parse dev_loss_tmo " "attribute '%s'", rport_id, value); goto out;
On Thu, Jul 02, 2020 at 02:31:17PM +0000, Martin Wilck wrote: > On Wed, 2020-07-01 at 17:39 -0500, Benjamin Marzinski wrote: > > dev_loss_tmo is a u32 value. However the kernel sysfs code prints it > > as > > a signed integer. This means that if dev_loss_tmo is above INT_MAX, > > the > > sysfs value will be a negative number. Parsing this was causing > > sysfs_set_rport_tmo() to fail. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > Thanks for catching this. I think it can be fixed simpler, because > strtoul() parses negative values just fine. See below. Sure. Lets go with that. -Ben > > Regards, > Martin > > >From 16eca9b0f340a13fee0c28ae52dffa578193f015 Mon Sep 17 00:00:00 2001 > From: Benjamin Marzinski <bmarzins@redhat.com> > Date: Wed, 1 Jul 2020 17:39:33 -0500 > Subject: [PATCH] libmultipath: fix sysfs dev_loss_tmo parsing > > dev_loss_tmo is a u32 value. However the kernel sysfs code prints it as > a signed integer. This means that if dev_loss_tmo is above INT_MAX, the > sysfs value will be a negative number. Parsing this was causing > sysfs_set_rport_tmo() to fail. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > Signed-off-by: Martin Wilck <mwilck@suse.com> > --- > libmultipath/discovery.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c > index 1d542ea..3c9803a 100644 > --- a/libmultipath/discovery.c > +++ b/libmultipath/discovery.c > @@ -581,7 +581,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp) > struct udev_device *rport_dev = NULL; > char value[16], *eptr; > char rport_id[32]; > - unsigned long long tmo = 0; > + unsigned int tmo; > int ret; > > sprintf(rport_id, "rport-%d:%d-%d", > @@ -605,8 +605,8 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp) > "error %d", rport_id, -ret); > goto out; > } > - tmo = strtoull(value, &eptr, 0); > - if (value == eptr || tmo == ULLONG_MAX) { > + tmo = strtoul(value, &eptr, 0); > + if (value == eptr) { > condlog(0, "%s: Cannot parse dev_loss_tmo " > "attribute '%s'", rport_id, value); > goto out; > -- > 2.26.2 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c index ffec5162..6a45979a 100644 --- a/libmultipath/discovery.c +++ b/libmultipath/discovery.c @@ -583,7 +583,8 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp) struct udev_device *rport_dev = NULL; char value[16], *eptr; char rport_id[32]; - unsigned long long tmo = 0; + long long ll_tmo; + unsigned int tmo = 0; int ret; sprintf(rport_id, "rport-%d:%d-%d", @@ -607,13 +608,13 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp) "error %d", rport_id, -ret); goto out; } - tmo = strtoull(value, &eptr, 0); - if (value == eptr || tmo == ULLONG_MAX) { + ll_tmo = strtoll(value, &eptr, 0); + if (value == eptr || ll_tmo > UINT_MAX || ll_tmo < INT_MIN) { condlog(0, "%s: Cannot parse dev_loss_tmo " "attribute '%s'", rport_id, value); goto out; } - + tmo = (unsigned int)ll_tmo; /* * This is tricky. * dev_loss_tmo will be limited to 600 if fast_io_fail
dev_loss_tmo is a u32 value. However the kernel sysfs code prints it as a signed integer. This means that if dev_loss_tmo is above INT_MAX, the sysfs value will be a negative number. Parsing this was causing sysfs_set_rport_tmo() to fail. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- libmultipath/discovery.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)