On Mon, Mar 02, 2020 at 09:19:33PM +0000, Chaitanya Kulkarni wrote:
> By any chance will the following patch be able to get rid of
> the warning ?
>
> index 4560878f0bac..889555910555 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -1895,9 +1895,7 @@ static ssize_t sysfs_blk_trace_attr_store(struct
> device *dev,
> goto out_unlock_bdev;
> }
>
> - ret = 0;
> - if (bt == NULL)
> - ret = blk_trace_setup_queue(q, bdev);
> + ret = bt == NULL ? blk_trace_setup_queue(q, bdev) : 0;
>
> if (ret == 0) {
> if (attr == &dev_attr_act_mask)
I don't have a way to run Coverity (I just get periodic reports), but
I don't see the point of this patch. I assume "bt" can still be NULL
(otherwise there'd be no point in testing it) and we still reference
"bt->pid" below.
> On 03/02/2020 12:41 PM, Bjorn Helgaas wrote:
> > On Thu, Feb 06, 2020 at 03:28:12PM +0100, Jan Kara wrote:
> >
> >> @@ -1844,18 +1896,18 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,
> >> }
> >>
> >> ret = 0;
> >> - if (q->blk_trace == NULL)
> >> + if (bt == NULL)
> >> ret = blk_trace_setup_queue(q, bdev);
> >>
> >> if (ret == 0) {
> >> if (attr == &dev_attr_act_mask)
> >> - q->blk_trace->act_mask = value;
> >> + bt->act_mask = value;
> >
> > You've likely seen this already, but Coverity complains that "bt" may
> > be a NULL pointer here, since we checked it for NULL above.
> >
> > CID 1460458: Null pointer dereferences (FORWARD_NULL)
> >
> >> else if (attr == &dev_attr_pid)
> >> - q->blk_trace->pid = value;
> >> + bt->pid = value;
> >> else if (attr == &dev_attr_start_lba)
> >> - q->blk_trace->start_lba = value;
> >> + bt->start_lba = value;
> >> else if (attr == &dev_attr_end_lba)
> >> - q->blk_trace->end_lba = value;
> >> + bt->end_lba = value;
> >> }
> >>
> >> out_unlock_bdev:
> >>
> >
>
