From: Dan Rosenberg Date: Fri, 12 Nov 2010 01:24:09 +0000 Subject: Re: [PATCH 3/10] Fix leaking of kernel heap addresses in net/ Message-Id: <1289525049.5167.82.camel@dan> List-Id: References: <1289524023.5167.67.camel@dan> <20101112012039.GB4683@canuck.infradead.org> In-Reply-To: <20101112012039.GB4683@canuck.infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Thomas Graf Cc: "David S. Miller" , Oliver Hartkopp , Alexey Kuznetsov , Urs Thuermann , Hideaki YOSHIFUJI , Patrick McHardy , James Morris , Remi Denis-Courmont , "Pekka Savola (ipv6)" , Sridhar Samudrala , Vlad Yasevich , Tejun Heo , Eric Dumazet , Li Zefan , Joe Perches , Stephen Hemminger , Jamal Hadi Salim , "Eric W. Biederman" , Alexey Dobriyan , Jiri Pirko , Johannes Berg , Daniel Lezcano , Pavel > > If we really have to do this. At least don't duplicate all this code. Do > the check in the printf argument: > > seq_printf(seq, "%4d: %08X:%04X %08X:%04X" > ... > capable(CAP_NET_ADMIN) ? sp : 0, > > I would even move the decision whether to expose kernel addresses or not > to a function so we can change behavior in one place. I wrote it this way because the format specifier must also be changed, or the %p output will print "(null)", which cannot be parsed by userspace programs expecting "(nil)" or 0. I could include another check inside the format specifier, but that seemed pretty ugly. But then again, it's ugly either way. -Dan