On 24/07/2019 11:02, Christoph Hellwig wrote:
> On Wed, Jul 24, 2019 at 09:48:38AM +0100, Steven Whitehouse wrote:
>> Hi,
>>
>> On 24/07/2019 09:43, Jia-Ju Bai wrote:
>>> In gfs2_alloc_inode(), when kmem_cache_alloc() on line 1724 returns
>>> NULL, ip is assigned to NULL. In this case, "return &ip->i_inode" will
>>> cause a null-pointer dereference.
>>>
>>> To fix this null-pointer dereference, NULL is returned when ip is NULL.
>>>
>>> This bug is found by a static analysis tool STCheck written by us.
>>
>> The bug is in the tool I'm afraid. Since i_inode is the first element of ip,
>> there is no NULL dereference here. A pointer to ip->i_inode and a pointer to
>> ip are one and the same (bar the differing types) which is the reason that
>> we return &ip->i_inode rather than just ip,
>
> But that doesn't help if ip is NULL, as dereferencing a field in in
> still remains invalid behavior.
>
According to C99 you may be right that it is undefined behaviour, that just
happens to work correctly on current versions
of GCC, see [1].
Although as pointed out in [1] this undefined behaviour was relied upon to
implement the offsetof macro
(nowadays that should be a compiler built-in, see include/linux/stddef.h).
I wish that cases like these would be more clearly defined in the C standard,
at least as implementation defined behavior such that GCC
could then be explicit about what semantics it wants and document it here:
https://gcc.gnu.org/onlinedocs/gcc/#toc-C-Implementation-Defined-Behavior
It is difficult to write future-proof code if the compiler can keep changing
its mind about semantics of code that it has previously accepted.
[1]
https://software.intel.com/en-us/blogs/2015/04/20/null-pointer-dereferencing-causes-undefined-behavior
Best regards,
--Edwin
