[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] remote_internal.c: don't dereference a NULL "conn"



Jim Meyering wrote:
> Daniel Veillard wrote:
>> On Wed, Sep 02, 2009 at 11:58:07AM +0200, Jim Meyering wrote:
>>> Here's the test just before the else-if in the patch below:
>>>
>>>     if (conn &&
>>>         conn->driver &&
>>>         STREQ (conn->driver->name, "remote")) {
>>>
>>> So, in the else-branch, "conn" is guaranteed to be NULL.
>>> And dereferenced.
>>>
>>> This may be only a theoretical risk, but if so,
>>> the test of "conn" above should be changed to an assertion,
>>> and/or the parameter should get the nonnull attribute.
>>
>>   I'm fine with your patch, so that even if code around changes
>> having a gard is IMHO a good thing
>
> Thanks for the quick review.
> Daniel Berrange said he'd prefer the nonnull approach I mentioned above,
> so I've just adjusted (caveat, still not tested or even compiled)
>
>>From 375bbb2d85fdf708858d726b6c7682e40dc07c2c Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering redhat com>
> Date: Wed, 2 Sep 2009 12:20:32 +0200
> Subject: [PATCH 1/2] infra: define ATTRIBUTE_NONNULL to mark non-NULL parameters
>
> * src/internal.h (ATTRIBUTE_NONNULL): Define.
...
>
>>From 61dc45fcae8b17d9f619563d427f1bc74f44d553 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering redhat com>
> Date: Wed, 2 Sep 2009 12:22:14 +0200
> Subject: [PATCH 2/2] remote_internal.c: appease clang
>
> * src/remote_internal.c (remoteNetworkOpen): Mark "conn" parameter
> as non-NULL.  Remove now-unnecessary "conn == NULL" test.
> (remoteDevMonOpen): Likewise.

Note that this patch now address two reported NULL-deref problems,
not just one, like the original.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]