Re: [PATCH] libmultipath: sanitize fd handling

From: Martin Wilck <mwilck@suse.com>
To: Hannes Reinecke <hare@suse.de>,
	Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: dm-devel@redhat.com, Hannes Reinecke <hare@suse.com>
Subject: Re: [PATCH] libmultipath: sanitize fd handling
Date: Wed, 20 Sep 2017 13:32:49 +0200	[thread overview]
Message-ID: <1505907169.4256.7.camel@suse.com> (raw)
In-Reply-To: <1505456112-117105-1-git-send-email-hare@suse.de>

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