linux-kernel - Re: [PATCH next V2] drivers: Use NULL not zero pointer assignments

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <4F233188.2010307@gmail.com>
Date:	Sat, 28 Jan 2012 10:21:44 +1100
From:	Ryan Mallon <rmallon@...il.com>
To:	Julia Lawall <julia.lawall@...6.fr>
CC:	Joe Perches <joe@...ches.com>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	Jiri Kosina <trivial@...nel.org>,
	kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH next V2] drivers: Use NULL not zero pointer assignments

On 28/01/12 09:46, Julia Lawall wrote:
> On Sat, 28 Jan 2012, Ryan Mallon wrote:
>
>> On 27/01/12 18:38, Joe Perches wrote:
>>> On Fri, 2012-01-27 at 17:20 +1100, Ryan Mallon wrote:
>>>> On 27/01/12 16:33, Joe Perches wrote:
>>>>> Using NULL pointer assignments is more kernel-style standard.
>>>>> Uncompiled, untested.
>>>>> Done via cocinelle script:
>>>>> @@
>>>>> type T;
>>>>> T *pointer;
>>>>> @@
>>>>> -pointer = 0
>>>>> +pointer = NULL
>>>> Hi Joe,
>>> Hi Ryan.
>>>
>>>> I think you can drop a lot of the initialisations completely rather than
>>>> convert them. I've pointed some out below. I'm just scanning through for
>>>> likely candidates and I didn't bother looking at the staging drivers, so
>>>> this list is not comprehensive.
>>> I'll try a more generic solution.
>>> Here's a possible cocci script that looks for declarations with
>>> initializations that are later overwritten.
>>
>> I think you need to be careful with this. I don't know enough about Coccinelle, but in general control flow makes this a difficult problem. This are also the cases where a variable is assigned on all control paths before being used, but also has an (seemingly useless) initialiser because gcc occasionally cannot determine that the variable is always initialised (because in general it is undecidable) and so the initialisation is there to prevent a compiler warning.
>
> Coccinelle follows control-flow paths.  But it is not likely to be more clever about this than gcc.  It is unaware of the value of anything, so it does not realize that some control-flow paths are impossible.  So I don't think there would be big problems here.
>
>> There were also a couple of cases I pointed out where the assignment of zero to a pointer variable was actually masking some other bug, and so replacing the assignment with NULL, or removing the initialisation is not necessarily the correct fix.
>
> But in this case it doesn't seem like it would hurt?  Having 0 or NULL would probably not make someone more or less likely to think about the function as a whole?

The reason not to do this is because it is just code churn: Replacing broken code with code that is broken in a slightly different way is not particularly constructive.

>
> For example, in the container_of case, having NULL is probably better than 0, because someone else is more likely to look for container_of's of NULL than container_of's of 0.

Possibly, I think it should be obvious in either case though that the code is probably not correct and needs to be fixed correctly.

~Ryan

--
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/