All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libmultipath: sanitize fd handling
@ 2017-09-15  6:15 Hannes Reinecke
  2017-09-20 11:32 ` Martin Wilck
  0 siblings, 1 reply; 3+ messages in thread
From: Hannes Reinecke @ 2017-09-15  6:15 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: dm-devel, Hannes Reinecke

One should remember that '0' _is_ a valid fd, so we need to set
the fd to '-1' upon allocating the path structure and ensure we're
checking for a value _smaller_ than 0 to detect an invalid fd.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/checkers.c | 4 ++--
 libmultipath/print.c    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index 05e024f8..c32f7274 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -146,7 +146,7 @@ struct checker * add_checker (char *multipath_dir, char * name)
 	if (!c->repair)
 		goto out;
 
-	c->fd = 0;
+	c->fd = -1;
 	c->sync = 1;
 	list_add(&c->node, &checkers);
 	return c;
@@ -237,7 +237,7 @@ int checker_check (struct checker * c)
 		MSG(c, "checker disabled");
 		return PATH_UNCHECKED;
 	}
-	if (c->fd <= 0) {
+	if (c->fd < 0) {
 		MSG(c, "no usable fd");
 		return PATH_WILD;
 	}
diff --git a/libmultipath/print.c b/libmultipath/print.c
index 95dff90b..65a98247 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -1677,7 +1677,7 @@ int snprint_status(char *buff, int len, struct vectors *vecs)
 	int monitored_count = 0;
 
 	vector_foreach_slot(vecs->pathvec, pp, i)
-		if (pp->fd != -1)
+		if (pp->fd >= 0)
 			monitored_count++;
 	fwd += snprintf(buff + fwd, len - fwd, "\npaths: %d\nbusy: %s\n",
 			monitored_count, is_uevent_busy()? "True" : "False");
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] libmultipath: sanitize fd handling
  2017-09-15  6:15 [PATCH] libmultipath: sanitize fd handling Hannes Reinecke
@ 2017-09-20 11:32 ` Martin Wilck
  2017-09-20 20:24   ` Martin Wilck
  0 siblings, 1 reply; 3+ messages in thread
From: Martin Wilck @ 2017-09-20 11:32 UTC (permalink / raw)
  To: Hannes Reinecke, Christophe Varoqui; +Cc: dm-devel, Hannes Reinecke

On Fri, 2017-09-15 at 08:15 +0200, Hannes Reinecke wrote:
> One should remember that '0' _is_ a valid fd, so we need to set
> the fd to '-1' upon allocating the path structure and ensure we're
> checking for a value _smaller_ than 0 to detect an invalid fd.
> 
> > diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
> > index 05e024f8..c32f7274 100644
> > --- a/libmultipath/checkers.c
> > +++ b/libmultipath/checkers.c
> > @@ -146,7 +146,7 @@ struct checker * add_checker (char
> > *multipath_dir, char * name)
> >  	if (!c->repair)
> >  		goto out;
> >  
> > -	c->fd = 0;
> > +	c->fd = -1;
> >  	c->sync = 1;
> >  	list_add(&c->node, &checkers);
> >  	return c;
> > @@ -237,7 +237,7 @@ int checker_check (struct checker * c)
> >  		MSG(c, "checker disabled");
> >  		return PATH_UNCHECKED;
> >  	}
> > -	if (c->fd <= 0) {
> > +	if (c->fd < 0) {
> >  		MSG(c, "no usable fd");
> >  		return PATH_WILD;
> >  	}
> > diff --git a/libmultipath/print.c b/libmultipath/print.c
> > index 95dff90b..65a98247 100644
> > --- a/libmultipath/print.c
> > +++ b/libmultipath/print.c
> > @@ -1677,7 +1677,7 @@ int snprint_status(char *buff, int len,
> struct
> > vectors *vecs)
> >  	int monitored_count = 0;
> >  
> >  	vector_foreach_slot(vecs->pathvec, pp, i)
> > -		if (pp->fd != -1)
> > +		if (pp->fd >= 0)
> >  			monitored_count++;
> >  	fwd += snprintf(buff + fwd, len - fwd, "\npaths: %d\nbusy:
> > %s\n",
> >  			monitored_count, is_uevent_busy()? "True"
> :
> > "False");
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  libmultipath/checkers.c | 4 ++--
>  libmultipath/print.c    | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 

There's one more wrong fd reference in
libmultipath/checkers/cciss_tur.c:76. And a nitpick: The comment in
checkers.h:14 should be fixed as well.

Apart from that: nice (although not strictly necessary, as multipathd
currently runs with fd 0 connected to /dev/null under normal
conditions).

Martin 
-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] libmultipath: sanitize fd handling
  2017-09-20 11:32 ` Martin Wilck
@ 2017-09-20 20:24   ` Martin Wilck
  0 siblings, 0 replies; 3+ messages in thread
From: Martin Wilck @ 2017-09-20 20:24 UTC (permalink / raw)
  To: Hannes Reinecke, Christophe Varoqui; +Cc: dm-devel, Hannes Reinecke

On Wed, 2017-09-20 at 13:32 +0200, Martin Wilck wrote:
> On Fri, 2017-09-15 at 08:15 +0200, Hannes Reinecke wrote:
> > 
> > Signed-off-by: Hannes Reinecke <hare@suse.com>
> > ---
> >  libmultipath/checkers.c | 4 ++--
> >  libmultipath/print.c    | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> 
> There's one more wrong fd reference in
> libmultipath/checkers/cciss_tur.c:76. And a nitpick: The comment in
> checkers.h:14 should be fixed as well.
> 
> Apart from that: nice (although not strictly necessary, as multipathd
> currently runs with fd 0 connected to /dev/null under normal
> conditions).
> 

I overlooked the memset usage in my review. Hannes himself brought that
to my attention. Will send fixup patch in a minute.

Martin 

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-09-20 20:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-15  6:15 [PATCH] libmultipath: sanitize fd handling Hannes Reinecke
2017-09-20 11:32 ` Martin Wilck
2017-09-20 20:24   ` Martin Wilck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.