* [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).