linux-kernel - Re: [PATCH v3 0/4] staging: r8188eu: add error handling of usb read errors

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 7 Jun 2022 13:07:39 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Pavel Skripkin <paskripkin@...il.com>
Cc:     Larry.Finger@...inger.net, phil@...lpotter.co.uk,
        straube.linux@...il.com, dan.carpenter@...cle.com,
        linux-kernel@...r.kernel.org, linux-staging@...ts.linux.dev
Subject: Re: [PATCH v3 0/4] staging: r8188eu: add error handling of usb read
 errors

On Mon, Jun 06, 2022 at 10:36:46PM +0300, Pavel Skripkin wrote:
> Hi,
> 
> it's reincarnation of my old series for adding sane error handling in
> r8818eu.
> 
> *Problem*
> 
> Old code was returning just stack variable in case of read error. It's
> not the best approach, since passing around stack data might cause
> device misconfiguration or even kernel data leakage
> 
> To solve this I've changed rtw_read{8,16,32} prototypes to return an error via
> return value and data via passed pointer. Some work should be done to
> propogate an error down to calltrace, but it's good way to at least
> start doing sane I/O error handling
> 
> Tested locally on qemu with TP-Link TL-WN722N v2/v3 [Realtek RTL8188EUS]
> device. More testing is welcomed, of course :)
> 
> _NOTE_
> Series is based on top of staging-testing branch.
> 
> Changes since v2:
>     get rid of `(void)res` and return an error from function instead of
>     hiding it
> 
> Changes since v1:
>     addresses issues found by Dan and self review. Mostly related to returning
>     _FAIL instead of -errno, since callers expect _FAIL/_SUCCESS
> 
> v1: https://lore.kernel.org/linux-staging/cover.1652911343.git.paskripkin@gmail.com/
> v2: https://lore.kernel.org/linux-staging/cover.1652994483.git.paskripkin@gmail.com/
> 
> Pavel Skripkin (4):
>   staging: r8188eu: add error handling of rtw_read8
>   staging: r8188eu: add error handling of rtw_read16
>   staging: r8188eu: add error handling of rtw_read32
>   MAINTAINERS: add myself as r8188eu reviewer
> 
>  MAINTAINERS                                   |   1 +
>  drivers/staging/r8188eu/core/rtw_cmd.c        |  15 +-
>  drivers/staging/r8188eu/core/rtw_efuse.c      |  33 ++-
>  drivers/staging/r8188eu/core/rtw_fw.c         |  72 +++--
>  drivers/staging/r8188eu/core/rtw_led.c        |  16 +-
>  drivers/staging/r8188eu/core/rtw_mlme_ext.c   |  62 ++++-
>  drivers/staging/r8188eu/core/rtw_pwrctrl.c    |   9 +-
>  drivers/staging/r8188eu/core/rtw_wlan_util.c  |  20 +-
>  .../r8188eu/hal/Hal8188ERateAdaptive.c        |  21 +-
>  drivers/staging/r8188eu/hal/HalPhyRf_8188e.c  |  21 +-
>  drivers/staging/r8188eu/hal/HalPwrSeqCmd.c    |   9 +-
>  drivers/staging/r8188eu/hal/hal_com.c         |  27 +-
>  drivers/staging/r8188eu/hal/rtl8188e_cmd.c    |  37 ++-
>  drivers/staging/r8188eu/hal/rtl8188e_dm.c     |   6 +-
>  .../staging/r8188eu/hal/rtl8188e_hal_init.c   | 136 +++++++---
>  drivers/staging/r8188eu/hal/rtl8188e_phycfg.c |  30 ++-
>  drivers/staging/r8188eu/hal/usb_halinit.c     | 251 +++++++++++++++---
>  drivers/staging/r8188eu/hal/usb_ops_linux.c   |  33 ++-
>  drivers/staging/r8188eu/include/rtw_io.h      |   6 +-
>  drivers/staging/r8188eu/os_dep/ioctl_linux.c  |  47 +++-
>  drivers/staging/r8188eu/os_dep/os_intfs.c     |  19 +-
>  21 files changed, 693 insertions(+), 178 deletions(-)

After applying, I get the following build warning which breaks the
build:

drivers/staging/r8188eu/core/rtw_mlme_ext.c:6826:13: error: unused variable ‘res’ [-Werror=unused-variable]
 6826 |         int res;
      |             ^~~

Please test-build your patches before sending them out :(

greg k-h