* Re: [PATCH] vDPA/ifcvf: match pointer check to use [not found] <20220315124130.1710030-1-trix@redhat.com> @ 2022-03-15 13:23 ` Michael S. Tsirkin 2022-03-15 13:28 ` Michael S. Tsirkin 1 sibling, 0 replies; 3+ messages in thread From: Michael S. Tsirkin @ 2022-03-15 13:23 UTC (permalink / raw) To: trix Cc: llvm, ndesaulniers, linux-kernel, virtualization, nathan, xieyongji, lingshan.zhu On Tue, Mar 15, 2022 at 05:41:30AM -0700, trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > Clang static analysis reports this issue > ifcvf_main.c:49:4: warning: Called function > pointer is null (null dereference) > vf->vring->cb.callback(vring->cb.private); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > The check > vring = &vf->vring[i]; > if (vring->cb.callback) > > Does not match the use. Change dereference so they match. > > Fixes: 79333575b8bd ("vDPA/ifcvf: implement shared IRQ feature") > Signed-off-by: Tom Rix <trix@redhat.com> > --- > drivers/vdpa/ifcvf/ifcvf_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > index 3b48e717e89f7..4366320fb68d3 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > @@ -46,7 +46,7 @@ static irqreturn_t ifcvf_vqs_reused_intr_handler(int irq, void *arg) > for (i = 0; i < vf->nr_vring; i++) { > vring = &vf->vring[i]; > if (vring->cb.callback) > - vf->vring->cb.callback(vring->cb.private); > + vring->cb.callback(vring->cb.private); > } > > return IRQ_HANDLED; Oh, absolutely. In fact vf->vring->cb.callback is just vf->vring[0].cb.callback so it's wrong for any ring except 0. Does not make sense. So how did it work in testing then? No idea. Zhu Lingshan, care to comment? > -- > 2.26.3 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] vDPA/ifcvf: match pointer check to use [not found] <20220315124130.1710030-1-trix@redhat.com> 2022-03-15 13:23 ` [PATCH] vDPA/ifcvf: match pointer check to use Michael S. Tsirkin @ 2022-03-15 13:28 ` Michael S. Tsirkin [not found] ` <512a392d-23d7-c25b-7576-571001f28288@redhat.com> 1 sibling, 1 reply; 3+ messages in thread From: Michael S. Tsirkin @ 2022-03-15 13:28 UTC (permalink / raw) To: trix Cc: llvm, ndesaulniers, linux-kernel, virtualization, nathan, xieyongji, lingshan.zhu On Tue, Mar 15, 2022 at 05:41:30AM -0700, trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > Clang static analysis reports this issue > ifcvf_main.c:49:4: warning: Called function > pointer is null (null dereference) > vf->vring->cb.callback(vring->cb.private); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > The check > vring = &vf->vring[i]; > if (vring->cb.callback) > > Does not match the use. Change dereference so they match. > > Fixes: 79333575b8bd ("vDPA/ifcvf: implement shared IRQ feature") Thanks a lot! I squashed this into the offending patch - no point in breaking bisect. Pushed to linux. However I'm now having second thoughts about applying that patchset - I'd like soma analysis explaining how this got through testing. > Signed-off-by: Tom Rix <trix@redhat.com> > --- > drivers/vdpa/ifcvf/ifcvf_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > index 3b48e717e89f7..4366320fb68d3 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > @@ -46,7 +46,7 @@ static irqreturn_t ifcvf_vqs_reused_intr_handler(int irq, void *arg) > for (i = 0; i < vf->nr_vring; i++) { > vring = &vf->vring[i]; > if (vring->cb.callback) > - vf->vring->cb.callback(vring->cb.private); > + vring->cb.callback(vring->cb.private); > } > > return IRQ_HANDLED; > -- > 2.26.3 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <512a392d-23d7-c25b-7576-571001f28288@redhat.com>]
* Re: [PATCH] vDPA/ifcvf: match pointer check to use [not found] ` <512a392d-23d7-c25b-7576-571001f28288@redhat.com> @ 2022-03-15 15:15 ` Michael S. Tsirkin 0 siblings, 0 replies; 3+ messages in thread From: Michael S. Tsirkin @ 2022-03-15 15:15 UTC (permalink / raw) To: Tom Rix Cc: llvm, ndesaulniers, linux-kernel, virtualization, nathan, xieyongji, lingshan.zhu On Tue, Mar 15, 2022 at 08:03:26AM -0700, Tom Rix wrote: > > On 3/15/22 6:28 AM, Michael S. Tsirkin wrote: > > On Tue, Mar 15, 2022 at 05:41:30AM -0700, trix@redhat.com wrote: > > > From: Tom Rix <trix@redhat.com> > > > > > > Clang static analysis reports this issue > > > ifcvf_main.c:49:4: warning: Called function > > > pointer is null (null dereference) > > > vf->vring->cb.callback(vring->cb.private); > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > The check > > > vring = &vf->vring[i]; > > > if (vring->cb.callback) > > > > > > Does not match the use. Change dereference so they match. > > > > > > Fixes: 79333575b8bd ("vDPA/ifcvf: implement shared IRQ feature") > > Thanks a lot! I squashed this into the offending patch - no point in > > breaking bisect. Pushed to linux. However I'm now > > having second thoughts about applying that patchset - I'd like > > soma analysis explaining how this got through testing. > > static analysis is something i do treewide. > > There are currently ~2500 issues in linux-next, do not panic! many are false > positives. > > It is pretty easy to setup and once you have a baseline you can filter only > your files. > > Tom Thanks for that info! I was actually directing this question to the contributor since the code does not look like it could have ever worked. I don't have the hardware in question myself. > > > Signed-off-by: Tom Rix <trix@redhat.com> > > > --- > > > drivers/vdpa/ifcvf/ifcvf_main.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > > > index 3b48e717e89f7..4366320fb68d3 100644 > > > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > > > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > > > @@ -46,7 +46,7 @@ static irqreturn_t ifcvf_vqs_reused_intr_handler(int irq, void *arg) > > > for (i = 0; i < vf->nr_vring; i++) { > > > vring = &vf->vring[i]; > > > if (vring->cb.callback) > > > - vf->vring->cb.callback(vring->cb.private); > > > + vring->cb.callback(vring->cb.private); > > > } > > > return IRQ_HANDLED; > > > -- > > > 2.26.3 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-03-15 15:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220315124130.1710030-1-trix@redhat.com>
2022-03-15 13:23 ` [PATCH] vDPA/ifcvf: match pointer check to use Michael S. Tsirkin
2022-03-15 13:28 ` Michael S. Tsirkin
[not found] ` <512a392d-23d7-c25b-7576-571001f28288@redhat.com>
2022-03-15 15:15 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).