2007-02-12 23:44:44

by NeilBrown

[permalink] [raw]
Subject: [PATCH 001 of 9] knfsd: nfsd4: fix non-terminated string


From: J. Bruce Fields <[email protected]>
The server name is expected to be a null-terminated string, so we can't
pass in the raw client identifier.

What's more, the client identifier is just a binary, not necessarily
printable, blob. Let's just use the ip address instead. The server
name appears to exist just to help debugging by making some printk's
more informative.

Note that the string is copies into the rpc client structure, so
the pointer to the local variable does not outlive the function call.

Signed-off-by: "J. Bruce Fields" <[email protected]>
Signed-off-by: Neil Brown <[email protected]>

### Diffstat output
./fs/nfsd/nfs4callback.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff .prev/fs/nfsd/nfs4callback.c ./fs/nfsd/nfs4callback.c
--- .prev/fs/nfsd/nfs4callback.c 2007-02-13 09:50:26.000000000 +1100
+++ ./fs/nfsd/nfs4callback.c 2007-02-13 10:00:59.000000000 +1100
@@ -387,7 +387,6 @@ nfsd4_probe_callback(struct nfs4_client
.address = (struct sockaddr *)&addr,
.addrsize = sizeof(addr),
.timeout = &timeparms,
- .servername = clp->cl_name.data,
.program = program,
.version = nfs_cb_version[1]->number,
.authflavor = RPC_AUTH_UNIX, /* XXX: need AUTH_GSS... */
@@ -397,6 +396,7 @@ nfsd4_probe_callback(struct nfs4_client
.rpc_proc = &nfs4_cb_procedures[NFSPROC4_CLNT_CB_NULL],
.rpc_argp = clp,
};
+ char clientname[16];
int status;

if (atomic_read(&cb->cb_set))
@@ -419,6 +419,11 @@ nfsd4_probe_callback(struct nfs4_client
memset(program->stats, 0, sizeof(cb->cb_stat));
program->stats->program = program;

+ /* Just here to make some printk's more useful: */
+ snprintf(clientname, sizeof(clientname),
+ "%u.%u.%u.%u", NIPQUAD(addr.sin_addr));
+ args.servername = clientname;
+
/* Create RPC client */
cb->cb_client = rpc_create(&args);
if (IS_ERR(cb->cb_client)) {

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2007-02-14 13:00:21

by Ming Zhang

[permalink] [raw]
Subject: Re: [PATCH 001 of 9] knfsd: nfsd4: fix non-terminated string

On Tue, 2007-02-13 at 10:44 +1100, NeilBrown wrote:
> From: J. Bruce Fields <[email protected]>
> The server name is expected to be a null-terminated string, so we can't
> pass in the raw client identifier.
>
> What's more, the client identifier is just a binary, not necessarily
> printable, blob. Let's just use the ip address instead. The server
> name appears to exist just to help debugging by making some printk's
> more informative.
>
> Note that the string is copies into the rpc client structure, so
> the pointer to the local variable does not outlive the function call.
>
> Signed-off-by: "J. Bruce Fields" <[email protected]>
> Signed-off-by: Neil Brown <[email protected]>
>
> ### Diffstat output
> ./fs/nfsd/nfs4callback.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff .prev/fs/nfsd/nfs4callback.c ./fs/nfsd/nfs4callback.c
> --- .prev/fs/nfsd/nfs4callback.c 2007-02-13 09:50:26.000000000 +1100
> +++ ./fs/nfsd/nfs4callback.c 2007-02-13 10:00:59.000000000 +1100
> @@ -387,7 +387,6 @@ nfsd4_probe_callback(struct nfs4_client
> .address = (struct sockaddr *)&addr,
> .addrsize = sizeof(addr),
> .timeout = &timeparms,
> - .servername = clp->cl_name.data,
> .program = program,
> .version = nfs_cb_version[1]->number,
> .authflavor = RPC_AUTH_UNIX, /* XXX: need AUTH_GSS... */
> @@ -397,6 +396,7 @@ nfsd4_probe_callback(struct nfs4_client
> .rpc_proc = &nfs4_cb_procedures[NFSPROC4_CLNT_CB_NULL],
> .rpc_argp = clp,
> };
> + char clientname[16];
> int status;
>
> if (atomic_read(&cb->cb_set))
> @@ -419,6 +419,11 @@ nfsd4_probe_callback(struct nfs4_client
> memset(program->stats, 0, sizeof(cb->cb_stat));
> program->stats->program = program;
>
> + /* Just here to make some printk's more useful: */
> + snprintf(clientname, sizeof(clientname),
> + "%u.%u.%u.%u", NIPQUAD(addr.sin_addr));

can use NIPQUAD_FMT here instead of "%u.%u.%u.%u".

btw, will the ip address here possibly be an ipv6 address?

> + args.servername = clientname;
> +
> /* Create RPC client */
> cb->cb_client = rpc_create(&args);
> if (IS_ERR(cb->cb_client)) {
>
> -------------------------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier.
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> NFS maillist - [email protected]
> https://lists.sourceforge.net/lists/listinfo/nfs
--
http://blackmagic02881.wordpress.com/


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-02-14 18:05:10

by Ming Zhang

[permalink] [raw]
Subject: Re: [PATCH 001 of 9] knfsd: nfsd4: fix non-terminated string

On Wed, 2007-02-14 at 09:55 -0800, Chuck Lever wrote:
> Ming Zhang wrote:
> > On Tue, 2007-02-13 at 10:44 +1100, NeilBrown wrote:
> >> From: J. Bruce Fields <[email protected]>
> >> The server name is expected to be a null-terminated string, so we can't
> >> pass in the raw client identifier.
> >>
> >> What's more, the client identifier is just a binary, not necessarily
> >> printable, blob. Let's just use the ip address instead. The server
> >> name appears to exist just to help debugging by making some printk's
> >> more informative.
> >>
> >> Note that the string is copies into the rpc client structure, so
> >> the pointer to the local variable does not outlive the function call.
> >>
> >> Signed-off-by: "J. Bruce Fields" <[email protected]>
> >> Signed-off-by: Neil Brown <[email protected]>
> >>
> >> ### Diffstat output
> >> ./fs/nfsd/nfs4callback.c | 7 ++++++-
> >> 1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff .prev/fs/nfsd/nfs4callback.c ./fs/nfsd/nfs4callback.c
> >> --- .prev/fs/nfsd/nfs4callback.c 2007-02-13 09:50:26.000000000 +1100
> >> +++ ./fs/nfsd/nfs4callback.c 2007-02-13 10:00:59.000000000 +1100
> >> @@ -387,7 +387,6 @@ nfsd4_probe_callback(struct nfs4_client
> >> .address = (struct sockaddr *)&addr,
> >> .addrsize = sizeof(addr),
> >> .timeout = &timeparms,
> >> - .servername = clp->cl_name.data,
> >> .program = program,
> >> .version = nfs_cb_version[1]->number,
> >> .authflavor = RPC_AUTH_UNIX, /* XXX: need AUTH_GSS... */
> >> @@ -397,6 +396,7 @@ nfsd4_probe_callback(struct nfs4_client
> >> .rpc_proc = &nfs4_cb_procedures[NFSPROC4_CLNT_CB_NULL],
> >> .rpc_argp = clp,
> >> };
> >> + char clientname[16];
> >> int status;
> >>
> >> if (atomic_read(&cb->cb_set))
> >> @@ -419,6 +419,11 @@ nfsd4_probe_callback(struct nfs4_client
> >> memset(program->stats, 0, sizeof(cb->cb_stat));
> >> program->stats->program = program;
> >>
> >> + /* Just here to make some printk's more useful: */
> >> + snprintf(clientname, sizeof(clientname),
> >> + "%u.%u.%u.%u", NIPQUAD(addr.sin_addr));
> >
> > can use NIPQUAD_FMT here instead of "%u.%u.%u.%u".
> >
> > btw, will the ip address here possibly be an ipv6 address?
>
> Some patches are in the works to build in IPv6 support. See the patch
> series at http://oss.oracle.com/~cel/linux-2.6/2.6.19/patches/

thanks for the info.

--
http://blackmagic02881.wordpress.com/


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs