| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Yt8Fj0ahBzL/T8Aj@equinox>
Date: Mon, 25 Jul 2022 22:05:19 +0100
From: Phillip Potter <phil@...lpotter.co.uk>
To: Philipp Hortmann <philipp.g.hortmann@...il.com>
Cc: gregkh@...uxfoundation.org, Larry.Finger@...inger.net,
paskripkin@...il.com, straube.linux@...il.com, martin@...ser.cx,
abdun.nihaal@...il.com, linux-staging@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: r8188eu: convert rtw_pwr_wakeup to correct
error code semantics
On Sun, Jul 24, 2022 at 08:39:42PM +0200, Philipp Hortmann wrote:
> On 7/24/22 18:30, Phillip Potter wrote:
> > Convert the rtw_pwr_wakeup function to use 0 on success and -EPERM on
> > error - in all places where we handle this response, we use either -1 or
> > -EPERM currently anyway, which are equivalent. Also, for other places
> > along the same call chain where we are using -1, use -EPERM.
> >
> > This gets the driver closer to removal of the non-standard _SUCCESS and
> > _FAIL definitions, which are inverted compared to the standard in-kernel
> > error code mechanism.
> >
> > Signed-off-by: Phillip Potter <phil@...lpotter.co.uk>
> > ---
> > drivers/staging/r8188eu/core/rtw_p2p.c | 4 +--
> > drivers/staging/r8188eu/core/rtw_pwrctrl.c | 10 +++---
> > drivers/staging/r8188eu/os_dep/ioctl_linux.c | 32 ++++++++++----------
> > 3 files changed, 23 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/staging/r8188eu/core/rtw_p2p.c b/drivers/staging/r8188eu/core/rtw_p2p.c
> > index c306aafa183b..bd654d4ff8b4 100644
> > --- a/drivers/staging/r8188eu/core/rtw_p2p.c
> > +++ b/drivers/staging/r8188eu/core/rtw_p2p.c
> > @@ -1888,7 +1888,7 @@ int rtw_p2p_enable(struct adapter *padapter, enum P2P_ROLE role)
> > if (role == P2P_ROLE_DEVICE || role == P2P_ROLE_CLIENT || role == P2P_ROLE_GO) {
> > /* leave IPS/Autosuspend */
> > - if (rtw_pwr_wakeup(padapter) == _FAIL) {
> > + if (rtw_pwr_wakeup(padapter)) {
> > ret = _FAIL;
> > goto exit;
> > }
> > @@ -1902,7 +1902,7 @@ int rtw_p2p_enable(struct adapter *padapter, enum P2P_ROLE role)
> > init_wifidirect_info(padapter, role);
> > } else if (role == P2P_ROLE_DISABLE) {
> > - if (rtw_pwr_wakeup(padapter) == _FAIL) {
> > + if (rtw_pwr_wakeup(padapter)) {
> > ret = _FAIL;
> > goto exit;
> > }
> > diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
> > index cf9020a73933..1cef3ef4d5f1 100644
> > --- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
> > +++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
> > @@ -381,24 +381,24 @@ int rtw_pwr_wakeup(struct adapter *padapter)
> > struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> > unsigned long timeout = jiffies + msecs_to_jiffies(3000);
> > unsigned long deny_time;
> > - int ret = _SUCCESS;
> > + int ret = 0;
> > while (pwrpriv->ps_processing && time_before(jiffies, timeout))
> > msleep(10);
> > /* I think this should be check in IPS, LPS, autosuspend functions... */
> > if (check_fwstate(pmlmepriv, _FW_LINKED)) {
> > - ret = _SUCCESS;
> > + ret = 0;
> > goto exit;
> > }
> > if (pwrpriv->rf_pwrstate == rf_off && ips_leave(padapter) == _FAIL) {
> > - ret = _FAIL;
> > + ret = -EPERM;
> > goto exit;
> > }
> > if (padapter->bDriverStopped || !padapter->bup || !padapter->hw_init_completed) {
> > - ret = _FAIL;
> > + ret = -EPERM;
> > goto exit;
> > }
> > @@ -439,7 +439,7 @@ int rtw_pm_set_ips(struct adapter *padapter, u8 mode)
> > return 0;
> > } else if (mode == IPS_NONE) {
> > rtw_ips_mode_req(pwrctrlpriv, mode);
> > - if ((padapter->bSurpriseRemoved == 0) && (rtw_pwr_wakeup(padapter) == _FAIL))
> > + if ((padapter->bSurpriseRemoved == 0) && rtw_pwr_wakeup(padapter))
> > return -EFAULT;
> > } else {
> > return -EINVAL;
> > diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > index 930bb4aea435..e0ae0c3c51f8 100644
> > --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > @@ -689,7 +689,7 @@ static int rtw_wx_set_mode(struct net_device *dev, struct iw_request_info *a,
> > - if (_FAIL == rtw_pwr_wakeup(padapter)) {
> > + if (rtw_pwr_wakeup(padapter)) {
> > ret = -EPERM;
> > goto exit;
> > }
> > @@ -933,13 +933,13 @@ static int rtw_wx_set_wap(struct net_device *dev,
> > - if (_FAIL == rtw_pwr_wakeup(padapter)) {
> > - ret = -1;
> > + if (rtw_pwr_wakeup(padapter)) {
> > + ret = -EPERM;
> > goto exit;
> > }
> > if (!padapter->bup) {
> > - ret = -1;
> > + ret = -EPERM;
> > goto exit;
> > }
> > @@ -1049,23 +1049,23 @@ static int rtw_wx_set_scan(struct net_device *dev, struct iw_request_info *a,
> > struct ndis_802_11_ssid ssid[RTW_SSID_SCAN_AMOUNT];
> > struct wifidirect_info *pwdinfo = &padapter->wdinfo;
> > - if (_FAIL == rtw_pwr_wakeup(padapter)) {
> > - ret = -1;
> > + if (rtw_pwr_wakeup(padapter)) {
> > + ret = -EPERM;
> > goto exit;
> > }
> > if (padapter->bDriverStopped) {
> > - ret = -1;
> > + ret = -EPERM;
> > goto exit;
> > }
> > if (!padapter->bup) {
> > - ret = -1;
> > + ret = -EPERM;
> > goto exit;
> > }
> > if (!padapter->hw_init_completed) {
> > - ret = -1;
> > + ret = -EPERM;
> > goto exit;
> > }
> > @@ -1164,7 +1164,7 @@ static int rtw_wx_set_scan(struct net_device *dev, struct iw_request_info *a,
> > }
> > if (!_status)
> > - ret = -1;
> > + ret = -EPERM;
> > exit:
> > @@ -1252,13 +1252,13 @@ static int rtw_wx_set_essid(struct net_device *dev,
> > uint ret = 0, len;
> > - if (_FAIL == rtw_pwr_wakeup(padapter)) {
> > - ret = -1;
> > + if (rtw_pwr_wakeup(padapter)) {
> > + ret = -EPERM;
> > goto exit;
> > }
> > if (!padapter->bup) {
> > - ret = -1;
> > + ret = -EPERM;
> > goto exit;
> > }
> > @@ -1268,7 +1268,7 @@ static int rtw_wx_set_essid(struct net_device *dev,
> > }
> > if (check_fwstate(pmlmepriv, WIFI_AP_STATE)) {
> > - ret = -1;
> > + ret = -EPERM;
> > goto exit;
> > }
> > @@ -1301,7 +1301,7 @@ static int rtw_wx_set_essid(struct net_device *dev,
> > }
> > if (!rtw_set_802_11_infrastructure_mode(padapter, pnetwork->network.InfrastructureMode)) {
> > - ret = -1;
> > + ret = -EPERM;
> > spin_unlock_bh(&queue->lock);
> > goto exit;
> > }
> > @@ -1312,7 +1312,7 @@ static int rtw_wx_set_essid(struct net_device *dev,
> > spin_unlock_bh(&queue->lock);
> > rtw_set_802_11_authentication_mode(padapter, authmode);
> > if (!rtw_set_802_11_ssid(padapter, &ndis_ssid)) {
> > - ret = -1;
> > + ret = -EPERM;
> > goto exit;
> > }
> > }
>
> Tested-by: Philipp Hortmann <philipp.g.hortmann@...il.com> # Edimax N150
Hi Philipp,
Thank you for testing, much appreciated. Need to do a V2 now, but wanted
to thank you anyway for your effort.
Regards,
Phil
Powered by blists - more mailing lists