On Mon, Jun 23, 2014 at 07:28:16PM -0400, Trond Myklebust wrote: > On Mon, Jun 23, 2014 at 6:10 PM, J. Bruce Fields <bfields@xxxxxxxxxx> wrote: > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > > index 2d305a1..56bdf4a 100644 > > --- a/fs/nfsd/nfs4xdr.c > > +++ b/fs/nfsd/nfs4xdr.c > > @@ -600,7 +600,18 @@ nfsd4_decode_create(struct nfsd4_compoundargs *argp, struct nfsd4_create *create > > READ_BUF(4); > > create->cr_linklen = be32_to_cpup(p++); > > READ_BUF(create->cr_linklen); > > - SAVEMEM(create->cr_linkname, create->cr_linklen); > > + /* > > + * The VFS will want a null-terminated string, and > > + * null-terminating in place isn't safe since this might > > + * end on a page boundary: > > + */ > > + create->cr_linkname = > > + kmalloc(create->cr_linklen + 1, GFP_KERNEL); > > + if (!create->cr_linkname) > > + return nfserr_jukebox; > > + memcpy(create->cr_linkname, p, create->cr_linklen); > > + create->cr_linkname[create->cr_linklen] = '\0'; > > + defer_free(argp, kfree, create->cr_linkname); > > break; > > case NF4BLK: > > case NF4CHR: > > Note that "defer_free()" does yet another allocation here in order to > make space for a small 'struct tmpbuf' structure. Unlike the first > allocation, there is no check for whether or not that second > allocation is successful above, so you can easily end up with a silent > memory leakage (ditto for a number of other callers of defer_free()). > > Looking around at all the other users, wouldn't it perhaps make sense > to replace defer_free() with a helper that does just a single > allocation for both the memory buffer and the struct tmpbuf? Yeah, thanks, working on it.... --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
