diff mbox

[v3] closing fd associated with /proc/fs/nfsd/export_features

Message ID 20110712.220109.419121694701200961.yamato@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Masatake YAMATO July 12, 2011, 1:01 p.m. UTC
Very sorry for posting again and again.
fd shuold be closed even if read is failed.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>


> Sorry, my patch has a serious bug.
> The value returned from read() is stored to fd.
> So fd should not be passed to close().
> 
> I rearranged the code as below.
> 
> Signed-off-by: Masatake YAMATO <yamato@redhat.com>
> 
> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> index c250383..744c01b 100644
> --- a/support/nfs/exports.c
> +++ b/support/nfs/exports.c
> @@ -784,9 +784,10 @@ struct export_features *get_export_features(void)
>  	fd = open(path, O_RDONLY);
>  	if (fd == -1)
>  		goto good;
> -	fd = read(fd, buf, 50);
> -	if (fd == -1)
> +	c = read(fd, buf, 50);
> +	if (c == -1)
>  		goto err;
> +	close(fd);
>  	c = sscanf(buf, "%x %x", &ef.flags, &ef.secinfo_flags);
>  	if (c != 2)
>  		goto err;
> 
> 
>> On Tue, Jul 12, 2011 at 11:47:00AM +0900, Masatake YAMATO wrote:
>>> A fd associated with /proc/fs/nfsd/export_features opened in get_export_features is 
>>> not closed.
>>> 
>>> 
>>> Signed-off-by: Masatake YAMATO <yamato@redhat.com>
>> Acked-by: J. Bruce Fields <bfields@redhat.com>
>>> 
>>> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
>>> index c250383..819a18a 100644
>>> --- a/support/nfs/exports.c
>>> +++ b/support/nfs/exports.c
>>> @@ -787,6 +787,7 @@ struct export_features *get_export_features(void)
>>>  	fd = read(fd, buf, 50);
>>>  	if (fd == -1)
>>>  		goto err;
>>> +	close(fd);
>>>  	c = sscanf(buf, "%x %x", &ef.flags, &ef.secinfo_flags);
>>>  	if (c != 2)
>>>  		goto err;
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Steve Dickson July 12, 2011, 2:03 p.m. UTC | #1
On 07/12/2011 09:01 AM, Masatake YAMATO wrote:
> Very sorry for posting again and again.
> fd shuold be closed even if read is failed.
> 
> Signed-off-by: Masatake YAMATO <yamato@redhat.com>
> 
> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> index c250383..c96500f 100644
> --- a/support/nfs/exports.c
> +++ b/support/nfs/exports.c
> @@ -784,8 +784,9 @@ struct export_features *get_export_features(void)
>  	fd = open(path, O_RDONLY);
>  	if (fd == -1)
>  		goto good;
> -	fd = read(fd, buf, 50);
> -	if (fd == -1)
> +	c = read(fd, buf, 50);
> +	close(fd);
> +	if (c == -1)
>  		goto err;
>  	c = sscanf(buf, "%x %x", &ef.flags, &ef.secinfo_flags);
>  	if (c != 2)
> 
Committed...

steved.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields July 12, 2011, 2:20 p.m. UTC | #2
On Tue, Jul 12, 2011 at 10:01:09PM +0900, Masatake YAMATO wrote:
> Very sorry for posting again and again.

Whoops, shows how carefully I was reviewing....

> fd shuold be closed even if read is failed.

Looks good to me.

--b.

> 
> Signed-off-by: Masatake YAMATO <yamato@redhat.com>
> 
> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> index c250383..c96500f 100644
> --- a/support/nfs/exports.c
> +++ b/support/nfs/exports.c
> @@ -784,8 +784,9 @@ struct export_features *get_export_features(void)
>  	fd = open(path, O_RDONLY);
>  	if (fd == -1)
>  		goto good;
> -	fd = read(fd, buf, 50);
> -	if (fd == -1)
> +	c = read(fd, buf, 50);
> +	close(fd);
> +	if (c == -1)
>  		goto err;
>  	c = sscanf(buf, "%x %x", &ef.flags, &ef.secinfo_flags);
>  	if (c != 2)
> 
> > Sorry, my patch has a serious bug.
> > The value returned from read() is stored to fd.
> > So fd should not be passed to close().
> > 
> > I rearranged the code as below.
> > 
> > Signed-off-by: Masatake YAMATO <yamato@redhat.com>
> > 
> > diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> > index c250383..744c01b 100644
> > --- a/support/nfs/exports.c
> > +++ b/support/nfs/exports.c
> > @@ -784,9 +784,10 @@ struct export_features *get_export_features(void)
> >  	fd = open(path, O_RDONLY);
> >  	if (fd == -1)
> >  		goto good;
> > -	fd = read(fd, buf, 50);
> > -	if (fd == -1)
> > +	c = read(fd, buf, 50);
> > +	if (c == -1)
> >  		goto err;
> > +	close(fd);
> >  	c = sscanf(buf, "%x %x", &ef.flags, &ef.secinfo_flags);
> >  	if (c != 2)
> >  		goto err;
> > 
> > 
> >> On Tue, Jul 12, 2011 at 11:47:00AM +0900, Masatake YAMATO wrote:
> >>> A fd associated with /proc/fs/nfsd/export_features opened in get_export_features is 
> >>> not closed.
> >>> 
> >>> 
> >>> Signed-off-by: Masatake YAMATO <yamato@redhat.com>
> >> Acked-by: J. Bruce Fields <bfields@redhat.com>
> >>> 
> >>> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> >>> index c250383..819a18a 100644
> >>> --- a/support/nfs/exports.c
> >>> +++ b/support/nfs/exports.c
> >>> @@ -787,6 +787,7 @@ struct export_features *get_export_features(void)
> >>>  	fd = read(fd, buf, 50);
> >>>  	if (fd == -1)
> >>>  		goto err;
> >>> +	close(fd);
> >>>  	c = sscanf(buf, "%x %x", &ef.flags, &ef.secinfo_flags);
> >>>  	if (c != 2)
> >>>  		goto err;
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/support/nfs/exports.c b/support/nfs/exports.c
index c250383..c96500f 100644
--- a/support/nfs/exports.c
+++ b/support/nfs/exports.c
@@ -784,8 +784,9 @@  struct export_features *get_export_features(void)
 	fd = open(path, O_RDONLY);
 	if (fd == -1)
 		goto good;
-	fd = read(fd, buf, 50);
-	if (fd == -1)
+	c = read(fd, buf, 50);
+	close(fd);
+	if (c == -1)
 		goto err;
 	c = sscanf(buf, "%x %x", &ef.flags, &ef.secinfo_flags);
 	if (c != 2)