[PATCH] libmultipath: Don't chop const strings

[PATCH] libmultipath: Don't chop const strings

[Benjamin Marzinski]

multipath was chopping sysfs attr strings in order to make sure that
ending whitespace didn't cause them to overflow their buffer. However
those strings were const strings. This patch makes multipath check how
much of the string length is ending whitespace, so that it can tell if
it will really overflow the buffer without chopping the const string.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/discovery.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 228ffd3..a6ff658 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2005 Mike Anderson
  */
 #include <stdio.h>
+#include <ctype.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <sys/ioctl.h>
@@ -143,6 +144,7 @@ path_discovery (vector pathvec, struct config * conf, int flag)
 extern ssize_t								\
 sysfs_get_##fname (struct udev_device * udev, char * buff, size_t len)	\
 {									\
+	int l;							\
 	const char * attr;						\
 	const char * devname;						\
 									\
@@ -157,12 +159,14 @@ sysfs_get_##fname (struct udev_device * udev, char * buff, size_t len)	\
 			devname, #fname);				\
 		return -ENXIO;						\
 	}								\
-	if (strchop(attr) > len) {					\
+	for (l = strlen(attr); l >= 1 && isspace(attr[l-1]); l--);	\
+	if (l > len) {							\
 		condlog(3, "%s: overflow in attribute %s",		\
 			devname, #fname);				\
 		return -EINVAL;						\
 	}								\
-	return strlcpy(buff, attr, len);				\
+	strlcpy(buff, attr, len);					\
+	return strchop(buff);						\
 }
 
 declare_sysfs_get_str(devtype);
-- 
1.8.4.2

Re: [PATCH] libmultipath: Don't chop const strings

[Hannes Reinecke]

On 02/12/2014 05:21 PM, Benjamin Marzinski wrote:
> multipath was chopping sysfs attr strings in order to make sure that
> ending whitespace didn't cause them to overflow their buffer. However
> those strings were const strings. This patch makes multipath check how
> much of the string length is ending whitespace, so that it can tell if
> it will really overflow the buffer without chopping the const string.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Yeah, you are correct.
I actually have a similar patch, but didn't manage to send it upstream.

So:

Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH] libmultipath: Don't chop const strings

[Christophe Varoqui]

Applied,
thanks.

On Wed, Feb 12, 2014 at 5:21 PM, Benjamin Marzinski <bmarzins@redhat.com> wrote:
> multipath was chopping sysfs attr strings in order to make sure that
> ending whitespace didn't cause them to overflow their buffer. However
> those strings were const strings. This patch makes multipath check how
> much of the string length is ending whitespace, so that it can tell if
> it will really overflow the buffer without chopping the const string.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/discovery.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 228ffd3..a6ff658 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -4,6 +4,7 @@
>   * Copyright (c) 2005 Mike Anderson
>   */
>  #include <stdio.h>
> +#include <ctype.h>
>  #include <unistd.h>
>  #include <fcntl.h>
>  #include <sys/ioctl.h>
> @@ -143,6 +144,7 @@ path_discovery (vector pathvec, struct config * conf, int flag)
>  extern ssize_t                                                         \
>  sysfs_get_##fname (struct udev_device * udev, char * buff, size_t len) \
>  {                                                                      \
> +       int l;                                                  \
>         const char * attr;                                              \
>         const char * devname;                                           \
>                                                                         \
> @@ -157,12 +159,14 @@ sysfs_get_##fname (struct udev_device * udev, char * buff, size_t len)    \
>                         devname, #fname);                               \
>                 return -ENXIO;                                          \
>         }                                                               \
> -       if (strchop(attr) > len) {                                      \
> +       for (l = strlen(attr); l >= 1 && isspace(attr[l-1]); l--);      \
> +       if (l > len) {                                                  \
>                 condlog(3, "%s: overflow in attribute %s",              \
>                         devname, #fname);                               \
>                 return -EINVAL;                                         \
>         }                                                               \
> -       return strlcpy(buff, attr, len);                                \
> +       strlcpy(buff, attr, len);                                       \
> +       return strchop(buff);                                           \
>  }
>
>  declare_sysfs_get_str(devtype);
> --
> 1.8.4.2
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel