| Message ID | 20210519141559.3031063-1-weiyongjun1@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] samples: vfio-mdev: fix error return code in mdpy_fb_probe() | expand |
On Wed, 19 May 2021 14:15:59 +0000 Wei Yongjun <weiyongjun1@huawei.com> wrote: > Fix to return negative error code -ENOMEM from the error handling > case instead of 0, as done elsewhere in this function. > > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > --- > samples/vfio-mdev/mdpy-fb.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/samples/vfio-mdev/mdpy-fb.c b/samples/vfio-mdev/mdpy-fb.c > index 21dbf63d6e41..d4abc0594dbd 100644 > --- a/samples/vfio-mdev/mdpy-fb.c > +++ b/samples/vfio-mdev/mdpy-fb.c > @@ -131,8 +131,10 @@ static int mdpy_fb_probe(struct pci_dev *pdev, > width, height); > > info = framebuffer_alloc(sizeof(struct mdpy_fb_par), &pdev->dev); > - if (!info) > + if (!info) { > + ret = -ENOMEM; > goto err_release_regions; > + } > pci_set_drvdata(pdev, info); > par = info->par; > > I think there's also a question of why the three 'return -EINVAL;' exit paths between here and the prior call to pci_request_regions() don't also take this goto. Thanks, Alex
On Wed, May 19, 2021 at 09:45:12AM -0600, Alex Williamson wrote: > On Wed, 19 May 2021 14:15:59 +0000 > Wei Yongjun <weiyongjun1@huawei.com> wrote: > > > Fix to return negative error code -ENOMEM from the error handling > > case instead of 0, as done elsewhere in this function. > > > > Reported-by: Hulk Robot <hulkci@huawei.com> > > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > > --- > > samples/vfio-mdev/mdpy-fb.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/samples/vfio-mdev/mdpy-fb.c b/samples/vfio-mdev/mdpy-fb.c > > index 21dbf63d6e41..d4abc0594dbd 100644 > > --- a/samples/vfio-mdev/mdpy-fb.c > > +++ b/samples/vfio-mdev/mdpy-fb.c > > @@ -131,8 +131,10 @@ static int mdpy_fb_probe(struct pci_dev *pdev, > > width, height); > > > > info = framebuffer_alloc(sizeof(struct mdpy_fb_par), &pdev->dev); > > - if (!info) > > + if (!info) { > > + ret = -ENOMEM; > > goto err_release_regions; > > + } > > pci_set_drvdata(pdev, info); > > par = info->par; > > > > > > I think there's also a question of why the three 'return -EINVAL;' exit > paths between here and the prior call to pci_request_regions() don't > also take this goto. Thanks, > Smatch catches one of these leaks... Which is weird that it would ignore the other error paths. Perhaps it was intentional? samples/vfio-mdev/mdpy-fb.c:135 mdpy_fb_probe() warn: missing error code 'ret' samples/vfio-mdev/mdpy-fb.c:189 mdpy_fb_probe() warn: 'pdev' not released on lines: 120. regards, dan carpenter
> On Wed, May 19, 2021 at 09:45:12AM -0600, Alex Williamson wrote: >> On Wed, 19 May 2021 14:15:59 +0000 >> Wei Yongjun <weiyongjun1@huawei.com> wrote: >> >>> Fix to return negative error code -ENOMEM from the error handling >>> case instead of 0, as done elsewhere in this function. >>> >>> Reported-by: Hulk Robot <hulkci@huawei.com> >>> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> >>> --- >>> samples/vfio-mdev/mdpy-fb.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/samples/vfio-mdev/mdpy-fb.c b/samples/vfio-mdev/mdpy-fb.c >>> index 21dbf63d6e41..d4abc0594dbd 100644 >>> --- a/samples/vfio-mdev/mdpy-fb.c >>> +++ b/samples/vfio-mdev/mdpy-fb.c >>> @@ -131,8 +131,10 @@ static int mdpy_fb_probe(struct pci_dev *pdev, >>> width, height); >>> >>> info = framebuffer_alloc(sizeof(struct mdpy_fb_par), &pdev->dev); >>> - if (!info) >>> + if (!info) { >>> + ret = -ENOMEM; >>> goto err_release_regions; >>> + } >>> pci_set_drvdata(pdev, info); >>> par = info->par; >>> >>> >> I think there's also a question of why the three 'return -EINVAL;' exit >> paths between here and the prior call to pci_request_regions() don't >> also take this goto. Thanks, >> > Smatch catches one of these leaks... Which is weird that it would > ignore the other error paths. Perhaps it was intentional? > > samples/vfio-mdev/mdpy-fb.c:135 mdpy_fb_probe() warn: missing error code 'ret' > samples/vfio-mdev/mdpy-fb.c:189 mdpy_fb_probe() warn: 'pdev' not released on lines: 120. The first one is found by coccinelle script, and I have no patterns to catch the second one now. It seems that smatch is more clever with this kind of issues. Regards, Wei Yongjun
diff --git a/samples/vfio-mdev/mdpy-fb.c b/samples/vfio-mdev/mdpy-fb.c index 21dbf63d6e41..d4abc0594dbd 100644 --- a/samples/vfio-mdev/mdpy-fb.c +++ b/samples/vfio-mdev/mdpy-fb.c @@ -131,8 +131,10 @@ static int mdpy_fb_probe(struct pci_dev *pdev, width, height); info = framebuffer_alloc(sizeof(struct mdpy_fb_par), &pdev->dev); - if (!info) + if (!info) { + ret = -ENOMEM; goto err_release_regions; + } pci_set_drvdata(pdev, info); par = info->par;
Fix to return negative error code -ENOMEM from the error handling case instead of 0, as done elsewhere in this function. Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> --- samples/vfio-mdev/mdpy-fb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)