[PATCH] staging: rtl8192u: Add null check in rtl8192_usb_initendpoints
There is an allocation for priv->rx_urb[16] has no null check, which may lead to a null pointer dereference. Signed-off-by: Dinghao Liu --- drivers/staging/rtl8192u/r8192U_core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c index 93676af98629..9fc4adc83d77 100644 --- a/drivers/staging/rtl8192u/r8192U_core.c +++ b/drivers/staging/rtl8192u/r8192U_core.c @@ -1608,6 +1608,8 @@ static short rtl8192_usb_initendpoints(struct net_device *dev) void *oldaddr, *newaddr; priv->rx_urb[16] = usb_alloc_urb(0, GFP_KERNEL); + if (!priv->rx_urb[16]) + return -ENOMEM; priv->oldaddr = kmalloc(16, GFP_KERNEL); if (!priv->oldaddr) return -ENOMEM; -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fwserial: Fix error handling in fwserial_create
When fw_core_add_address_handler() fails, we need to destroy
the port by tty_port_destroy(). Also we need to unregister
the address handler by fw_core_remove_address_handler() on
failure.
Signed-off-by: Dinghao Liu
---
drivers/staging/fwserial/fwserial.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/fwserial/fwserial.c
b/drivers/staging/fwserial/fwserial.c
index db83d34cd677..c368082aae1a 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -2189,6 +2189,7 @@ static int fwserial_create(struct fw_unit *unit)
err = fw_core_add_address_handler(>rx_handler,
_high_memory_region);
if (err) {
+ tty_port_destroy(>port);
kfree(port);
goto free_ports;
}
@@ -2271,6 +2272,7 @@ static int fwserial_create(struct fw_unit *unit)
free_ports:
for (--i; i >= 0; --i) {
+ fw_core_remove_address_handler(>ports[i]->rx_handler);
tty_port_destroy(>ports[i]->port);
kfree(serial->ports[i]);
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] vme: ca91cx42: fix memleak in ca91cx42_dma_list_add
When we encounter invalid data width or address space, entry should be freed just like what we've done in the previous error paths. Signed-off-by: Dinghao Liu --- drivers/vme/bridges/vme_ca91cx42.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/vme/bridges/vme_ca91cx42.c b/drivers/vme/bridges/vme_ca91cx42.c index ea938dc29c5e..1c464afd9d4e 100644 --- a/drivers/vme/bridges/vme_ca91cx42.c +++ b/drivers/vme/bridges/vme_ca91cx42.c @@ -1100,7 +1100,8 @@ static int ca91cx42_dma_list_add(struct vme_dma_list *list, break; default: dev_err(dev, "Invalid data width\n"); - return -EINVAL; + retval = -EINVAL; + goto err_cycle; } /* Setup address space */ @@ -1122,8 +1123,8 @@ static int ca91cx42_dma_list_add(struct vme_dma_list *list, break; default: dev_err(dev, "Invalid address space\n"); - return -EINVAL; - break; + retval = -EINVAL; + goto err_cycle; } if (vme_attr->cycle & VME_SUPER) -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: atomisp: fix memleak in ia_css_stream_create
When aspect_ratio_crop_init() fails, curr_stream needs
to be freed just like what we've done in the following
error paths. However, current code is returning directly
and ends up leaking memory.
Signed-off-by: Dinghao Liu
---
drivers/staging/media/atomisp/pci/sh_css.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c
b/drivers/staging/media/atomisp/pci/sh_css.c
index 54434c2dbaf9..8473e1437074 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -9521,7 +9521,7 @@ ia_css_stream_create(const struct ia_css_stream_config
*stream_config,
if (err)
{
IA_CSS_LEAVE_ERR(err);
- return err;
+ goto ERR;
}
#endif
for (i = 0; i < num_pipes; i++)
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [v2] Staging: rtl8188eu: rtw_mlme: Fix uninitialized variable authmode
The variable authmode can be uninitialized. The danger would be if
it equals to _WPA_IE_ID_ (0xdd) or _WPA2_IE_ID_ (0x33). We can avoid
this by setting it to zero instead. This is the approach that was
used in the rtl8723bs driver.
Fixes: 7b464c9fa5cc ("staging: r8188eu: Add files for new driver - part 4")
Co-developed-by: Dan Carpenter
Signed-off-by: Dan Carpenter
Signed-off-by: Dinghao Liu
---
Changelog:
v2: - Move the initialization after 'else' statement.
Refine commit message.
---
drivers/staging/rtl8188eu/core/rtw_mlme.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 9de2d421f6b1..4f2abe1e14d5 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -1729,9 +1729,11 @@ int rtw_restruct_sec_ie(struct adapter *adapter, u8
*in_ie, u8 *out_ie, uint in_
if ((ndisauthmode == Ndis802_11AuthModeWPA) ||
(ndisauthmode == Ndis802_11AuthModeWPAPSK))
authmode = _WPA_IE_ID_;
- if ((ndisauthmode == Ndis802_11AuthModeWPA2) ||
+ else if ((ndisauthmode == Ndis802_11AuthModeWPA2) ||
(ndisauthmode == Ndis802_11AuthModeWPA2PSK))
authmode = _WPA2_IE_ID_;
+ else
+ authmode = 0x0;
if (check_fwstate(pmlmepriv, WIFI_UNDER_WPS)) {
memcpy(out_ie + ielength, psecuritypriv->wps_ie,
psecuritypriv->wps_ie_len);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: [PATCH] Staging: rtl8188eu: rtw_mlme: Fix uninitialized variable authmode
> I review things in the order that they appear in my inbox so I hadn't
> seen Greg and Larry's comments. You've now stumbled into an area of
> politics where you have conflicting reviews... :P Fortunately, we're
> all of us reasonable people.
>
> I think your patch is correct in that it is what the original coder
> intended. You just need to sell the patch correctly in the commit
> message. The real danger of the original code would be if "authmode" is
> accidentally 0x30 or 0xdd just because it was uninitialized. Setting it
> to zero ensures that it is not and it also matches how this is handled
> in the rtl8723bs driver. This matches the original author's intention.
>
> So:
>
> 1) Re-write the commit message.
>
> The variable authmode can be uninitialized. The danger would be
> if it equals _WPA_IE_ID_ (0xdd) or _WPA2_IE_ID_ (0x33). We can
> avoid this by setting it to zero instead. This is the approach that
> was used in the rtl8723bs driver.
>
> 2) Add a fixes tag.
>Fixes: 7b464c9fa5cc ("staging: r8188eu: Add files for new driver - part 4")
>
> 3) Change the commit to Larry's style with the "else if" and "else".
>Sometimes people just disagree about style so it's easiest to go with
>whatever the maintainer (Larry) wants. It's not worth debating one
>way or the other so just redo it.
>
> Then resend. Google for "how to send a v2 patch" to get the right
> format.
>
> regards,
> dan carpenter
>
Your advice is very helpful to me, thanks! I will prepare v2 patch and
resend it soon.
Regards,
Dinghao
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: [PATCH] Staging: rtl8188eu: rtw_mlme: Fix uninitialized variable authmode
> > Yes, in this routine, it would be possible for authmode to not be set; > however, > later code only compares it to either _WPA_IE_ID_ or _WPA2_IE_ID_. It is > never > used in a way that an unset value could make the program flow be different by > arbitrarily setting the value to zero. Thus your statement "Then authmode may > contain a garbage value and influence the execution flow of this function." > is > false. > It's clear to me. Thank you for your advice. Regards, Dinghao ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: rtl8188eu: rtw_mlme: Fix uninitialized variable authmode
The variable authmode will keep uninitialized if neither if
statements used to initialize this variable are not triggered.
Then authmode may contain a garbage value and influence the
execution flow of this function.
Fix this by initializing it to zero.
Signed-off-by: Dinghao Liu
---
drivers/staging/rtl8188eu/core/rtw_mlme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 9de2d421f6b1..716f8d8a5c13 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -1711,7 +1711,7 @@ static int rtw_append_pmkid(struct adapter *Adapter, int
iEntry, u8 *ie, uint ie
int rtw_restruct_sec_ie(struct adapter *adapter, u8 *in_ie, u8 *out_ie, uint
in_len)
{
- u8 authmode;
+ u8 authmode = 0;
uint ielength;
int iEntry;
struct mlme_priv *pmlmepriv = >mlmepriv;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: Re: [PATCH] [v2] media: staging: tegra-vde: fix runtime pm imbalance on error
Sorry, I misunderstood your idea before. A new function is the best solution for this problem. Regards, Dinghao Dan Carpenter dan.carpen...@oracle.com写道： > On Thu, May 21, 2020 at 07:42:56PM +0800, dinghao@zju.edu.cn wrote: > > We need to make sure if pm_runtime_get_sync() is designed with > > such behavior before modifying it. > > > > I received a response from Rafael when I commited a similar patch: > > https://lkml.org/lkml/2020/5/20/1100 > > It seems that this behavior is intentional and needs to be kept. > > Yes. This is why I have said twice or three times to not change > pm_runtime_get_sync() but instead to write a replacement. > > A large percent of the callers are buggy. The pm_runtime_get_sync() is > a -4 on Rusty's API scale. > http://sweng.the-davies.net/Home/rustys-api-design-manifesto > One could argue that anything above a -4 is really a 2 if you read > the implementation thouroughly enough... > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: [PATCH] [v2] media: staging: tegra-vde: fix runtime pm imbalance on error
We need to make sure if pm_runtime_get_sync() is designed with such behavior before modifying it. I received a response from Rafael when I commited a similar patch: https://lkml.org/lkml/2020/5/20/1100 It seems that this behavior is intentional and needs to be kept. Regards, Dinghao Dan Carpenter dan.carpen...@oracle.com写道： > On Thu, May 21, 2020 at 02:27:45PM +0800, Dinghao Liu wrote: > > pm_runtime_get_sync() increments the runtime PM usage counter even > > the call returns an error code. Thus a pairing decrement is needed > > on the error handling path to keep the counter balanced. > > > > Signed-off-by: Dinghao Liu > > Let's stop working around the bug in pm_runtime_get_sync() and write > a replacement for it instead. > > regards, > dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [v2] media: staging: tegra-vde: fix runtime pm imbalance on error
pm_runtime_get_sync() increments the runtime PM usage counter even the call returns an error code. Thus a pairing decrement is needed on the error handling path to keep the counter balanced. Signed-off-by: Dinghao Liu --- Changelog: v2: - Remove unused label 'unlock' --- drivers/staging/media/tegra-vde/vde.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c index d3e63512a765..3fdf2cd0b99e 100644 --- a/drivers/staging/media/tegra-vde/vde.c +++ b/drivers/staging/media/tegra-vde/vde.c @@ -777,7 +777,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde, ret = pm_runtime_get_sync(dev); if (ret < 0) - goto unlock; + goto put_runtime_pm; /* * We rely on the VDE registers reset value, otherwise VDE @@ -843,8 +843,6 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde, put_runtime_pm: pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); - -unlock: mutex_unlock(>lock); release_dpb_frames: -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: [PATCH] media: staging: tegra-vde: fix runtime pm imbalance on error
Hi, Dan,
I agree the best solution is to fix __pm_runtime_resume(). But there are also
many cases that assume pm_runtime_get_sync() will change PM usage
counter on error. According to my static analysis results, the number of these
"right" cases are larger. Adjusting __pm_runtime_resume() directly will
introduce
more new bugs. Therefore I think we should resolve the "bug" cases individually.
I think that Dmitry's patch is more reasonable than mine.
Dinghao
Dan Carpenter dan.carpen...@oracle.com写道：
> On Wed, May 20, 2020 at 01:15:44PM +0300, Dmitry Osipenko wrote:
> > 20.05.2020 12:51, Dinghao Liu пишет:
> > > pm_runtime_get_sync() increments the runtime PM usage counter even
> > > it returns an error code. Thus a pairing decrement is needed on
> > > the error handling path to keep the counter balanced.
> > >
> > > Signed-off-by: Dinghao Liu
> > > ---
> > > drivers/staging/media/tegra-vde/vde.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/media/tegra-vde/vde.c
> > > b/drivers/staging/media/tegra-vde/vde.c
> > > index d3e63512a765..dd134a3a15c7 100644
> > > --- a/drivers/staging/media/tegra-vde/vde.c
> > > +++ b/drivers/staging/media/tegra-vde/vde.c
> > > @@ -777,7 +777,7 @@ static int tegra_vde_ioctl_decode_h264(struct
> > > tegra_vde *vde,
> > >
> > > ret = pm_runtime_get_sync(dev);
> > > if (ret < 0)
> > > - goto unlock;
> > > + goto put_runtime_pm;
> > >
> > > /*
> > >* We rely on the VDE registers reset value, otherwise VDE
> > >
> >
> > Hello Dinghao,
> >
> > Thank you for the patch. I sent out a similar patch a week ago [1].
> >
> > [1]
> > https://patchwork.ozlabs.org/project/linux-tegra/patch/20200514210847.9269-2-dig...@gmail.com/
> >
> > The pm_runtime_put_noidle() should have the same effect as yours
> > variant, although my variant won't change the last_busy RPM time, which
> > I think is a bit more appropriate behavior.
>
> I don't think either patch is correct. The right thing to do is to fix
> __pm_runtime_resume() so it doesn't leak a reference count on error.
>
> The problem is that a lot of functions don't check the return so
> possibly we are relying on that behavior. We may need to introduce a
> new function which cleans up properly instead of leaking reference
> counts?
>
> Also it's not documented that pm_runtime_get_sync() returns 1 sometimes
> on success so it leads to a few bugs.
>
> drivers/gpu/drm/stm/ltdc.c: ret = pm_runtime_get_sync(ddev->dev);
> drivers/gpu/drm/stm/ltdc.c- if (ret) {
> --
> drivers/gpu/drm/stm/ltdc.c: ret = pm_runtime_get_sync(ddev->dev);
> drivers/gpu/drm/stm/ltdc.c- if (ret) {
>
> drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c: ret =
> pm_runtime_get_sync(pm->dev);
> drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c- if (ret)
>
> drivers/media/platform/ti-vpe/cal.c:ret = pm_runtime_get_sync(>dev);
> drivers/media/platform/ti-vpe/cal.c-if (ret)
>
> drivers/mfd/arizona-core.c: ret =
> pm_runtime_get_sync(arizona->dev);
> drivers/mfd/arizona-core.c- if (ret != 0)
>
> drivers/remoteproc/qcom_q6v5_adsp.c:ret = pm_runtime_get_sync(adsp->dev);
> drivers/remoteproc/qcom_q6v5_adsp.c-if (ret)
>
> drivers/spi/spi-img-spfi.c: ret = pm_runtime_get_sync(dev);
> drivers/spi/spi-img-spfi.c- if (ret)
>
> drivers/usb/dwc3/dwc3-pci.c:ret = pm_runtime_get_sync(>dev);
> drivers/usb/dwc3/dwc3-pci.c-if (ret)
>
> drivers/watchdog/rti_wdt.c: ret = pm_runtime_get_sync(dev);
> drivers/watchdog/rti_wdt.c- if (ret) {
>
> regards,
> dan carpenter
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 99c7da112c95..e280991a977d 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -1082,6 +1082,9 @@ int __pm_runtime_resume(struct device *dev, int
> rpmflags)
> retval = rpm_resume(dev, rpmflags);
> spin_unlock_irqrestore(>power.lock, flags);
>
> + if (retval < 0 && rpmflags & RPM_GET_PUT)
> + atomic_dec(>power.usage_count);
> +
> return retval;
> }
> EXPORT_SYMBOL_GPL(__pm_runtime_resume);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging: tegra-vde: fix runtime pm imbalance on error
pm_runtime_get_sync() increments the runtime PM usage counter even it returns an error code. Thus a pairing decrement is needed on the error handling path to keep the counter balanced. Signed-off-by: Dinghao Liu --- drivers/staging/media/tegra-vde/vde.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c index d3e63512a765..dd134a3a15c7 100644 --- a/drivers/staging/media/tegra-vde/vde.c +++ b/drivers/staging/media/tegra-vde/vde.c @@ -777,7 +777,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde, ret = pm_runtime_get_sync(dev); if (ret < 0) - goto unlock; + goto put_runtime_pm; /* * We rely on the VDE registers reset value, otherwise VDE -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
