* net/9p/mux.c: use-after-free
@ 2007-07-23 1:20 Adrian Bunk
2007-07-25 18:43 ` Eric Van Hensbergen
0 siblings, 1 reply; 4+ messages in thread
From: Adrian Bunk @ 2007-07-23 1:20 UTC (permalink / raw)
To: Latchesar Ionkov, Eric Van Hensbergen
Cc: v9fs-developer, netdev, linux-kernel
The Coverity checker spotted the following use-after-free
in net/9p/mux.c:
<-- snip -->
...
struct p9_conn *p9_conn_create(struct p9_transport *trans, int msize,
unsigned char *extended)
{
...
if (!m->tagpool) {
kfree(m);
return ERR_PTR(PTR_ERR(m->tagpool));
}
...
<-- snip -->
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: net/9p/mux.c: use-after-free
2007-07-23 1:20 net/9p/mux.c: use-after-free Adrian Bunk
@ 2007-07-25 18:43 ` Eric Van Hensbergen
2007-07-25 19:13 ` Latchesar Ionkov
0 siblings, 1 reply; 4+ messages in thread
From: Eric Van Hensbergen @ 2007-07-25 18:43 UTC (permalink / raw)
To: Adrian Bunk; +Cc: Latchesar Ionkov, v9fs-developer, netdev, linux-kernel
On 7/22/07, Adrian Bunk <bunk@stusta.de> wrote:
> The Coverity checker spotted the following use-after-free
> in net/9p/mux.c:
>
> <-- snip -->
>
> ...
> struct p9_conn *p9_conn_create(struct p9_transport *trans, int msize,
> unsigned char *extended)
> {
> ...
> if (!m->tagpool) {
> kfree(m);
> return ERR_PTR(PTR_ERR(m->tagpool));
> }
> ...
>
> <-- snip -->
>
I've got a fix for this one:
if (!m->tagpool) {
mtmp = ERR_PTR(PTR_ERR(m->tagpool));
kfree(m);
return mtmp;
}
but I was wondering about one of the other returns further down the function:
...
memset(&m->poll_waddr, 0, sizeof(m->poll_waddr));
m->poll_task = NULL;
n = p9_mux_poll_start(m);
if (n)
return ERR_PTR(n);
n = trans->poll(trans, &m->pt);
...
lucho: doesn't that constitute a leak? Shouldn't we be doing:
if (n) {
kfree(m);
return ERR_PTR(n);
}
-eric
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: net/9p/mux.c: use-after-free
2007-07-25 18:43 ` Eric Van Hensbergen
@ 2007-07-25 19:13 ` Latchesar Ionkov
2007-07-25 19:45 ` Eric Van Hensbergen
0 siblings, 1 reply; 4+ messages in thread
From: Latchesar Ionkov @ 2007-07-25 19:13 UTC (permalink / raw)
To: Eric Van Hensbergen; +Cc: Adrian Bunk, v9fs-developer, netdev, linux-kernel
Yep, it's a leak.
Thanks,
Lucho
On 7/25/07, Eric Van Hensbergen <ericvh@gmail.com> wrote:
> On 7/22/07, Adrian Bunk <bunk@stusta.de> wrote:
> > The Coverity checker spotted the following use-after-free
> > in net/9p/mux.c:
> >
> > <-- snip -->
> >
> > ...
> > struct p9_conn *p9_conn_create(struct p9_transport *trans, int msize,
> > unsigned char *extended)
> > {
> > ...
> > if (!m->tagpool) {
> > kfree(m);
> > return ERR_PTR(PTR_ERR(m->tagpool));
> > }
> > ...
> >
> > <-- snip -->
> >
>
> I've got a fix for this one:
> if (!m->tagpool) {
> mtmp = ERR_PTR(PTR_ERR(m->tagpool));
> kfree(m);
> return mtmp;
> }
>
> but I was wondering about one of the other returns further down the function:
>
> ...
> memset(&m->poll_waddr, 0, sizeof(m->poll_waddr));
> m->poll_task = NULL;
> n = p9_mux_poll_start(m);
> if (n)
> return ERR_PTR(n);
>
> n = trans->poll(trans, &m->pt);
> ...
>
> lucho: doesn't that constitute a leak? Shouldn't we be doing:
>
> if (n) {
> kfree(m);
> return ERR_PTR(n);
> }
>
> -eric
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2007-07-25 19:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-23 1:20 net/9p/mux.c: use-after-free Adrian Bunk
2007-07-25 18:43 ` Eric Van Hensbergen
2007-07-25 19:13 ` Latchesar Ionkov
2007-07-25 19:45 ` Eric Van Hensbergen
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).