linux-kernel - Re: [PATCH] Fix use-after-free of q->root_blkg and q->root

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121018133149.GA18147@redhat.com>
Date:	Thu, 18 Oct 2012 09:31:49 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	"Jun'ichi Nomura" <j-nomura@...jp.nec.com>
Cc:	Tejun Heo <tj@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Fix use-after-free of q->root_blkg and q->root_rl.blkg

On Thu, Oct 18, 2012 at 11:56:34AM +0900, Jun'ichi Nomura wrote:

[..]
> >>>         if (ent == &q->root_blkg->q_node)
> >>
> >> So ent is not &q->root_blkg->q_node.
> > 
> > If q->root_blkg is NULL, will it not lead to NULL pointer dereference.
> > (q->root_blkg->q_node).
> 
> It's not dereferenced.

Ok. We are taking address of root_blkg->q_node so even if root_blkg=NULL,
address is just offset from null. Little subtle for me. :-)

> 
> >>>                 ent = ent->next;
> >>>         if (ent == &q->blkg_list)
> >>>                 return NULL;
> >>
> >> And we return NULL here.
> >>
> >> Ah, yes. You are correct.
> >> We can do without the above hunk.
> > 
> > I would rather prefer to check for this boundary condition early and
> > return instead of letting it fall through all these conditions and
> > then figure out yes we have no next rl. IMO, code becomes easier to
> > understand if nothing else. Otherwise one needs a step by step 
> > explanation as above to show that case of q->root_blkg is covered.
> 
> I have same opinion as yours that it's good for readability.


Tejun, for the sake of readability, are you fine with keeping the original
check and original patch which I had acked.

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/