re: media: rcar-vin: add group allocator functions

re: media: rcar-vin: add group allocator functions

[Colin Ian King]

Hi there,

Static analysis with CoverityScan picked up an explicit null pointer
dereference in the the following commit:

commit 3bb4c3bc85bf77a76c921671800bde2e1bf82a88
Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Date:   Sat Apr 14 07:57:18 2018 -0400

    media: rcar-vin: add group allocator functions

in:

+static void rvin_group_put(struct rvin_dev *vin)
+{
+       mutex_lock(&vin->group->lock);
+
+       vin->group = NULL;

vin->group is now NULL


+       vin->v4l2_dev.mdev = NULL;
+
+       if (WARN_ON(vin->group->vin[vin->id] != vin))
+               goto out;

vin->group is being dereferenced (null pointer dereference)

+
+       vin->group->vin[vin->id] = NULL;

vin->group is being dereferenced (null pointer dereference)

+out:
+       mutex_unlock(&vin->group->lock);
+
+       kref_put(&vin->group->refcount, rvin_group_release);
+}

So I think this needs fixing up.

Regards,

Colin

Re: media: rcar-vin: add group allocator functions

[Niklas Söderlund]

Hi Colin,

Thanks for reporting this. I wonder why smatch and sparse did not catch 
this, the fault can't be mine for writing such a obviously bad thing 
right :-)

I have a patch to address this, just need to test it before posting.

On 2018-04-24 14:14:02 +0100, Colin Ian King wrote:
> Hi there,
> 
> While running static analysis on linux-next today a null pointer
> dereference issue was detected by CoverityScan. The following commit
> introduced the issue:
> 
> commit 3bb4c3bc85bf77a76c921671800bde2e1bf82a88
> Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Date:   Sat Apr 14 07:57:18 2018 -0400
> 
>     media: rcar-vin: add group allocator functions
> 
> The analysis is as follows:
> 
> 339 static void rvin_group_put(struct rvin_dev *vin)
> 340 {
> 341        mutex_lock(&vin->group->lock);
> 342
>    1. assign_zero: Assigning: vin->group = NULL.
> 
> 343        vin->group = NULL;
> 344        vin->v4l2_dev.mdev = NULL;
> 345
>     CID 1468359 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
>     2. var_deref_op: Dereferencing null pointer vin->group.
> 
> 346        if (WARN_ON(vin->group->vin[vin->id] != vin))
> 347                goto out;
> 348
> 
> vin->group is NULL however the WARN_ON is dereferencing it, causing an
> OOPS.  I don't know how this should be fixed, hence I am sending this
> bug report.
> 
> Regards,
> 
> Colin

-- 
Regards,
Niklas Söderlund

re: media: rcar-vin: add group allocator functions

[Colin Ian King]

Hi there,

While running static analysis on linux-next today a null pointer
dereference issue was detected by CoverityScan. The following commit
introduced the issue:

commit 3bb4c3bc85bf77a76c921671800bde2e1bf82a88
Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Date:   Sat Apr 14 07:57:18 2018 -0400

    media: rcar-vin: add group allocator functions

The analysis is as follows:

339 static void rvin_group_put(struct rvin_dev *vin)
340 {
341        mutex_lock(&vin->group->lock);
342
   1. assign_zero: Assigning: vin->group = NULL.

343        vin->group = NULL;
344        vin->v4l2_dev.mdev = NULL;
345
    CID 1468359 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
    2. var_deref_op: Dereferencing null pointer vin->group.

346        if (WARN_ON(vin->group->vin[vin->id] != vin))
347                goto out;
348

vin->group is NULL however the WARN_ON is dereferencing it, causing an
OOPS.  I don't know how this should be fixed, hence I am sending this
bug report.

Regards,

Colin