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? Cheers Trond -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@xxxxxxxxxxxxxxx -- 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
