Messages in this thread | | | From | Christian Schoenebeck <> | Subject | Re: [PATCH] net: 9p: fix possible memory leak in p9_check_errors() | Date | Thu, 26 Oct 2023 15:18:16 +0200 |
| |
On Thursday, October 26, 2023 1:53:55 PM CEST asmadeus@codewreck.org wrote: > > Hangyu Hua wrote on Thu, Oct 26, 2023 at 05:23:51PM +0800: > > When p9pdu_readf is called with "s?d" attribute, it allocates a pointer > > that will store a string. But when p9pdu_readf() fails while handling "d" > > then this pointer will not be freed in p9_check_errors. > > Right, that sounds correct to me. > > Out of curiosity how did you notice this? The leak shouldn't happen with > any valid server. > > This cannot break anything so I'll push this to -next tomorrow and > submit to Linus next week > > > Fixes: ca41bb3e21d7 ("[net/9p] Handle Zero Copy TREAD/RERROR case in !dotl case.") > > This commit moves this code a bit, but the p9pdu_readf call predates > it -- in this case the Fixes tag is probably not useful; this affects > all maintained kernels.
Looks like it exists since introduction of p9_check_errors(), therefore:
Fixes: 51a87c552dfd ("9p: rework client code to use new protocol support functions")
> > Signed-off-by: Hangyu Hua <hbh25y@gmail.com> > > --- > > net/9p/client.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/net/9p/client.c b/net/9p/client.c > > index 86bbc7147fc1..6c7cd765b714 100644 > > --- a/net/9p/client.c > > +++ b/net/9p/client.c > > @@ -540,12 +540,15 @@ static int p9_check_errors(struct p9_client *c, struct p9_req_t *req) > > return 0; > > > > if (!p9_is_proto_dotl(c)) { > > - char *ename; > > + char *ename = NULL; > > > > err = p9pdu_readf(&req->rc, c->proto_version, "s?d", > > &ename, &ecode); > > - if (err) > > + if (err) { > > + if (ename != NULL) > > + kfree(ename); > > Don't check for NULL before kfree - kfree does it. > If that's the only remark you get I can fix it when applying the commit > on my side.
With those two remarks addressed:
Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> > > > goto out_err; > > + } > > > > if (p9_is_proto_dotu(c) && ecode < 512) > > err = -ecode; > >
| |