linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] closing fd associated with /proc/fs/nfsd/export_features
@ 2011-07-12  2:47 Masatake YAMATO
  2011-07-12 10:46 ` J. Bruce Fields
  0 siblings, 1 reply; 6+ messages in thread
From: Masatake YAMATO @ 2011-07-12  2:47 UTC (permalink / raw)
  To: linux-nfs

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>

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;

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] closing fd associated with /proc/fs/nfsd/export_features
  2011-07-12  2:47 [PATCH] closing fd associated with /proc/fs/nfsd/export_features Masatake YAMATO
@ 2011-07-12 10:46 ` J. Bruce Fields
  2011-07-12 12:38   ` [PATCH v2] " Masatake YAMATO
  0 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2011-07-12 10:46 UTC (permalink / raw)
  To: Masatake YAMATO; +Cc: linux-nfs, steved

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2] closing fd associated with /proc/fs/nfsd/export_features
  2011-07-12 10:46 ` J. Bruce Fields
@ 2011-07-12 12:38   ` Masatake YAMATO
  2011-07-12 13:01     ` [PATCH v3] " Masatake YAMATO
  0 siblings, 1 reply; 6+ messages in thread
From: Masatake YAMATO @ 2011-07-12 12:38 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, steved

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3] closing fd associated with /proc/fs/nfsd/export_features
  2011-07-12 12:38   ` [PATCH v2] " Masatake YAMATO
@ 2011-07-12 13:01     ` Masatake YAMATO
  2011-07-12 14:03       ` Steve Dickson
  2011-07-12 14:20       ` J. Bruce Fields
  0 siblings, 2 replies; 6+ messages in thread
From: Masatake YAMATO @ 2011-07-12 13:01 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, steved

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)

> 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] closing fd associated with /proc/fs/nfsd/export_features
  2011-07-12 13:01     ` [PATCH v3] " Masatake YAMATO
@ 2011-07-12 14:03       ` Steve Dickson
  2011-07-12 14:20       ` J. Bruce Fields
  1 sibling, 0 replies; 6+ messages in thread
From: Steve Dickson @ 2011-07-12 14:03 UTC (permalink / raw)
  To: Masatake YAMATO; +Cc: bfields, linux-nfs



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.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] closing fd associated with /proc/fs/nfsd/export_features
  2011-07-12 13:01     ` [PATCH v3] " Masatake YAMATO
  2011-07-12 14:03       ` Steve Dickson
@ 2011-07-12 14:20       ` J. Bruce Fields
  1 sibling, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2011-07-12 14:20 UTC (permalink / raw)
  To: Masatake YAMATO; +Cc: linux-nfs, steved

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-07-12 14:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-12  2:47 [PATCH] closing fd associated with /proc/fs/nfsd/export_features Masatake YAMATO
2011-07-12 10:46 ` J. Bruce Fields
2011-07-12 12:38   ` [PATCH v2] " Masatake YAMATO
2011-07-12 13:01     ` [PATCH v3] " Masatake YAMATO
2011-07-12 14:03       ` Steve Dickson
2011-07-12 14:20       ` J. Bruce Fields

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).