* [PATCH 4/4] NFSD: Ignore and warn once for more fslocs or more uuids
@ 2014-05-23 12:01 Kinglong Mee
2014-05-23 13:54 ` J. Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Kinglong Mee @ 2014-05-23 12:01 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Linux NFS Mailing List, kinglongmee
If mountd writes more fslocs or more uuids, just ignore and warn once.
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
fs/nfsd/export.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 90d37b6..1d56ae3 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -394,6 +394,12 @@ fsloc_parse(char **mesg, char *buf, struct nfsd4_fs_locations *fsloc)
int len;
int migrated, i, err;
+ /* utils writes more fslocs than one */
+ if (fsloc->locations) {
+ WARN_ON_ONCE(1);
+ return 0;
+ }
+
/* listsize */
err = get_uint(mesg, &fsloc->locations_count);
if (err)
@@ -485,6 +491,12 @@ uuid_parse(char **mesg, char *buf, unsigned char **puuid)
{
int len;
+ /* utils writes more uuid than one */
+ if (*puuid) {
+ WARN_ON_ONCE(1);
+ return 0;
+ }
+
/* expect a 16 byte uuid encoded as \xXXXX... */
len = qword_get(mesg, buf, PAGE_SIZE);
if (len != EX_UUID_LEN)
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] NFSD: Ignore and warn once for more fslocs or more uuids
2014-05-23 12:01 [PATCH 4/4] NFSD: Ignore and warn once for more fslocs or more uuids Kinglong Mee
@ 2014-05-23 13:54 ` J. Bruce Fields
2014-05-23 14:24 ` Kinglong Mee
0 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2014-05-23 13:54 UTC (permalink / raw)
To: Kinglong Mee; +Cc: Linux NFS Mailing List
On Fri, May 23, 2014 at 08:01:47PM +0800, Kinglong Mee wrote:
> If mountd writes more fslocs or more uuids, just ignore and warn once.
So the current behavior is a leak?
Have you observed any version of mountd actually writing more than one
of these?
If not, I'd prefer to return an error.
--b.
>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
> fs/nfsd/export.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 90d37b6..1d56ae3 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -394,6 +394,12 @@ fsloc_parse(char **mesg, char *buf, struct nfsd4_fs_locations *fsloc)
> int len;
> int migrated, i, err;
>
> + /* utils writes more fslocs than one */
> + if (fsloc->locations) {
> + WARN_ON_ONCE(1);
> + return 0;
> + }
> +
> /* listsize */
> err = get_uint(mesg, &fsloc->locations_count);
> if (err)
> @@ -485,6 +491,12 @@ uuid_parse(char **mesg, char *buf, unsigned char **puuid)
> {
> int len;
>
> + /* utils writes more uuid than one */
> + if (*puuid) {
> + WARN_ON_ONCE(1);
> + return 0;
> + }
> +
> /* expect a 16 byte uuid encoded as \xXXXX... */
> len = qword_get(mesg, buf, PAGE_SIZE);
> if (len != EX_UUID_LEN)
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] NFSD: Ignore and warn once for more fslocs or more uuids
2014-05-23 13:54 ` J. Bruce Fields
@ 2014-05-23 14:24 ` Kinglong Mee
2014-05-23 14:41 ` J. Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Kinglong Mee @ 2014-05-23 14:24 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Linux NFS Mailing List
On 5/23/2014 21:54, J. Bruce Fields wrote:
> On Fri, May 23, 2014 at 08:01:47PM +0800, Kinglong Mee wrote:
>> If mountd writes more fslocs or more uuids, just ignore and warn once.
>
> So the current behavior is a leak?
Yes, there is a memory leak for more fslocs or more uuids.
>
> Have you observed any version of mountd actually writing more than one
> of these?
Sorry, i don't checkout it.
I just review the codes, and rewrit nfs-utils to writing more fslocs or uuids.
>
> If not, I'd prefer to return an error.
You mean return -EINVAL instead of ignoring it with return 0?
Although, i miss checking the more "secinfo" in secinfo_parse() right now,
for no memory leak. I will adds it after decides how to fix above.
thanks,
Kinglong Mee
>
> --b.
>
>>
>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>> ---
>> fs/nfsd/export.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
>> index 90d37b6..1d56ae3 100644
>> --- a/fs/nfsd/export.c
>> +++ b/fs/nfsd/export.c
>> @@ -394,6 +394,12 @@ fsloc_parse(char **mesg, char *buf, struct nfsd4_fs_locations *fsloc)
>> int len;
>> int migrated, i, err;
>>
>> + /* utils writes more fslocs than one */
>> + if (fsloc->locations) {
>> + WARN_ON_ONCE(1);
>> + return 0;
>> + }
>> +
>> /* listsize */
>> err = get_uint(mesg, &fsloc->locations_count);
>> if (err)
>> @@ -485,6 +491,12 @@ uuid_parse(char **mesg, char *buf, unsigned char **puuid)
>> {
>> int len;
>>
>> + /* utils writes more uuid than one */
>> + if (*puuid) {
>> + WARN_ON_ONCE(1);
>> + return 0;
>> + }
>> +
>> /* expect a 16 byte uuid encoded as \xXXXX... */
>> len = qword_get(mesg, buf, PAGE_SIZE);
>> if (len != EX_UUID_LEN)
>> --
>> 1.9.0
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] NFSD: Ignore and warn once for more fslocs or more uuids
2014-05-23 14:24 ` Kinglong Mee
@ 2014-05-23 14:41 ` J. Bruce Fields
2014-05-24 3:19 ` [PATCH v2] NFSD: Error out when getting more than one fsloc/secinfo/uuid Kinglong Mee
0 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2014-05-23 14:41 UTC (permalink / raw)
To: Kinglong Mee; +Cc: Linux NFS Mailing List
On Fri, May 23, 2014 at 10:24:26PM +0800, Kinglong Mee wrote:
> On 5/23/2014 21:54, J. Bruce Fields wrote:
> > On Fri, May 23, 2014 at 08:01:47PM +0800, Kinglong Mee wrote:
> >> If mountd writes more fslocs or more uuids, just ignore and warn once.
> >
> > So the current behavior is a leak?
>
> Yes, there is a memory leak for more fslocs or more uuids.
>
> >
> > Have you observed any version of mountd actually writing more than one
> > of these?
>
> Sorry, i don't checkout it.
> I just review the codes, and rewrit nfs-utils to writing more fslocs or uuids.
>
> >
> > If not, I'd prefer to return an error.
>
> You mean return -EINVAL instead of ignoring it with return 0?
Right. If somebody introduces weird mountd behavior I'd rather they see
it fail right away. And also I think it's a little easier to extend
these calls later if we error out on invalid input.
Also, remove the WARN_ON_ONCE. We usually reserve BUG or WARN for cases
where we believe the kernel itself has a bug, and avoid them in cases
where they could be triggered by userland. (Admittedly mountd is very
special userland, but still I'd rather it not be able to trigger these.)
--b.
>
> Although, i miss checking the more "secinfo" in secinfo_parse() right now,
> for no memory leak. I will adds it after decides how to fix above.
>
> thanks,
> Kinglong Mee
>
> >
> > --b.
> >
> >>
> >> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> >> ---
> >> fs/nfsd/export.c | 12 ++++++++++++
> >> 1 file changed, 12 insertions(+)
> >>
> >> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> >> index 90d37b6..1d56ae3 100644
> >> --- a/fs/nfsd/export.c
> >> +++ b/fs/nfsd/export.c
> >> @@ -394,6 +394,12 @@ fsloc_parse(char **mesg, char *buf, struct nfsd4_fs_locations *fsloc)
> >> int len;
> >> int migrated, i, err;
> >>
> >> + /* utils writes more fslocs than one */
> >> + if (fsloc->locations) {
> >> + WARN_ON_ONCE(1);
> >> + return 0;
> >> + }
> >> +
> >> /* listsize */
> >> err = get_uint(mesg, &fsloc->locations_count);
> >> if (err)
> >> @@ -485,6 +491,12 @@ uuid_parse(char **mesg, char *buf, unsigned char **puuid)
> >> {
> >> int len;
> >>
> >> + /* utils writes more uuid than one */
> >> + if (*puuid) {
> >> + WARN_ON_ONCE(1);
> >> + return 0;
> >> + }
> >> +
> >> /* expect a 16 byte uuid encoded as \xXXXX... */
> >> len = qword_get(mesg, buf, PAGE_SIZE);
> >> if (len != EX_UUID_LEN)
> >> --
> >> 1.9.0
> >>
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] NFSD: Error out when getting more than one fsloc/secinfo/uuid
2014-05-23 14:41 ` J. Bruce Fields
@ 2014-05-24 3:19 ` Kinglong Mee
2014-05-27 15:47 ` J. Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Kinglong Mee @ 2014-05-24 3:19 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Linux NFS Mailing List, kinglongmee
v2:
remove WARN_ON_ONCE(1) and return -EINVAL instead of ignoring it.
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
fs/nfsd/export.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 7884051..13b85f9 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -389,6 +389,10 @@ fsloc_parse(char **mesg, char *buf, struct nfsd4_fs_locations *fsloc)
int len;
int migrated, i, err;
+ /* more than one fsloc */
+ if (fsloc->locations)
+ return -EINVAL;
+
/* listsize */
err = get_uint(mesg, &fsloc->locations_count);
if (err)
@@ -442,6 +446,10 @@ static int secinfo_parse(char **mesg, char *buf, struct svc_export *exp)
u32 listsize;
int err;
+ /* more than one secinfo */
+ if (exp->ex_nflavors)
+ return -EINVAL;
+
err = get_uint(mesg, &listsize);
if (err)
return err;
@@ -481,6 +489,10 @@ uuid_parse(char **mesg, char *buf, unsigned char **puuid)
{
int len;
+ /* more than one uuid */
+ if (*puuid)
+ return -EINVAL;
+
/* expect a 16 byte uuid encoded as \xXXXX... */
len = qword_get(mesg, buf, PAGE_SIZE);
if (len != EX_UUID_LEN)
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] NFSD: Error out when getting more than one fsloc/secinfo/uuid
2014-05-24 3:19 ` [PATCH v2] NFSD: Error out when getting more than one fsloc/secinfo/uuid Kinglong Mee
@ 2014-05-27 15:47 ` J. Bruce Fields
0 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2014-05-27 15:47 UTC (permalink / raw)
To: Kinglong Mee; +Cc: Linux NFS Mailing List
On Sat, May 24, 2014 at 11:19:57AM +0800, Kinglong Mee wrote:
> v2:
> remove WARN_ON_ONCE(1) and return -EINVAL instead of ignoring it.
Thanks, applying.--b.
>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
> fs/nfsd/export.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 7884051..13b85f9 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -389,6 +389,10 @@ fsloc_parse(char **mesg, char *buf, struct nfsd4_fs_locations *fsloc)
> int len;
> int migrated, i, err;
>
> + /* more than one fsloc */
> + if (fsloc->locations)
> + return -EINVAL;
> +
> /* listsize */
> err = get_uint(mesg, &fsloc->locations_count);
> if (err)
> @@ -442,6 +446,10 @@ static int secinfo_parse(char **mesg, char *buf, struct svc_export *exp)
> u32 listsize;
> int err;
>
> + /* more than one secinfo */
> + if (exp->ex_nflavors)
> + return -EINVAL;
> +
> err = get_uint(mesg, &listsize);
> if (err)
> return err;
> @@ -481,6 +489,10 @@ uuid_parse(char **mesg, char *buf, unsigned char **puuid)
> {
> int len;
>
> + /* more than one uuid */
> + if (*puuid)
> + return -EINVAL;
> +
> /* expect a 16 byte uuid encoded as \xXXXX... */
> len = qword_get(mesg, buf, PAGE_SIZE);
> if (len != EX_UUID_LEN)
> --
> 1.9.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-27 15:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-23 12:01 [PATCH 4/4] NFSD: Ignore and warn once for more fslocs or more uuids Kinglong Mee
2014-05-23 13:54 ` J. Bruce Fields
2014-05-23 14:24 ` Kinglong Mee
2014-05-23 14:41 ` J. Bruce Fields
2014-05-24 3:19 ` [PATCH v2] NFSD: Error out when getting more than one fsloc/secinfo/uuid Kinglong Mee
2014-05-27 15:47 ` 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).