linux-kernel - [PATCH v3 0/6] staging: r8188eu: avoid uninit value bugs

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <cover.1629789580.git.paskripkin@gmail.com>
Date:   Tue, 24 Aug 2021 10:25:45 +0300
From:   Pavel Skripkin <paskripkin@...il.com>
To:     Larry.Finger@...inger.net, phil@...lpotter.co.uk,
        gregkh@...uxfoundation.org, straube.linux@...il.com,
        fmdefrancesco@...il.com
Cc:     linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Pavel Skripkin <paskripkin@...il.com>
Subject: [PATCH v3 0/6] staging: r8188eu: avoid uninit value bugs

Hi, Greg, Larry and Phillip!

I noticed, that new staging driver was added like 3 weeks ago and I decided
to look at the code, because drivers in staging directory are always buggy.

The first thing I noticed is *no one* was checking read operations result, but
it can fail and driver may start writing random stack values into registers. It
can cause driver misbehavior or device misbehavior.

To avoid this type of bugs, i've changed rtw_read* API. Now all rtw_read
funtions return an error, when something went wrong with usb transfer.

It helps callers to break/return earlier and don't write random values to
registers or to rely on random values.


v2 -> v3:
  1. Fixed OOPS in usb_read32(), caused by writing to u32 **
  2. Fixed style in rtw_read32, rtw_read16 and rtw_read8 (Suggested by Dan)
  3. Added error hanling when usb_control_msg() returns ret != len
     NOTE: Dan suggested to add this to usbctrl_vendorreq(), but there is
     pending series, which will get rid of usb_control_msg(), so (res != len)
     check can be removed, when Fabio's series will go in
  4. Removed RFC tag

v1 -> v2:
  1. Make rtw_read*() return an error instead of initializing pointer to error
  2. Split one huge patch to smaller ones for each rtw_read{8,16,32} function
     changes
  3. Add new macro for printing register values (It helps to not copy-paste error
     handling)
  4. Removed {read,write}_macreg (Suggested by Phillip)
  5. Rebased on top of staging-next
  6. Cleaned checkpatch errors and warnings


Phillip has tested fixed v2 version, AFAIU

Pavel Skripkin (6):
  staging: r8188eu: remove {read,write}_macreg
  staging: r8188eu: add helper macro for printing registers
  staging: r8188eu: add error handling of rtw_read8
  staging: r8188eu: add error handling of rtw_read16
  staging: r8188eu: add error handling of rtw_read32
  staging: r8188eu: make ReadEFuse return an int

 drivers/staging/r8188eu/core/rtw_debug.c      |  79 +++-
 drivers/staging/r8188eu/core/rtw_efuse.c      | 125 +++--
 drivers/staging/r8188eu/core/rtw_io.c         |  27 +-
 drivers/staging/r8188eu/core/rtw_mp.c         |  70 ++-
 drivers/staging/r8188eu/core/rtw_mp_ioctl.c   |  13 +-
 drivers/staging/r8188eu/core/rtw_pwrctrl.c    |   5 +-
 drivers/staging/r8188eu/core/rtw_sreset.c     |   9 +-
 .../r8188eu/hal/Hal8188ERateAdaptive.c        |   8 +-
 drivers/staging/r8188eu/hal/HalPhyRf_8188e.c  |  21 +-
 drivers/staging/r8188eu/hal/HalPwrSeqCmd.c    |   9 +-
 drivers/staging/r8188eu/hal/hal_com.c         |  23 +-
 drivers/staging/r8188eu/hal/hal_intf.c        |   6 +-
 drivers/staging/r8188eu/hal/odm_interface.c   |  12 +-
 drivers/staging/r8188eu/hal/rtl8188e_cmd.c    |  33 +-
 drivers/staging/r8188eu/hal/rtl8188e_dm.c     |   6 +-
 .../staging/r8188eu/hal/rtl8188e_hal_init.c   | 285 +++++++++---
 drivers/staging/r8188eu/hal/rtl8188e_phycfg.c |  27 +-
 drivers/staging/r8188eu/hal/rtl8188e_sreset.c |  22 +-
 drivers/staging/r8188eu/hal/rtl8188eu_led.c   |  18 +-
 drivers/staging/r8188eu/hal/usb_halinit.c     | 439 +++++++++++++++---
 drivers/staging/r8188eu/hal/usb_ops_linux.c   |  62 ++-
 drivers/staging/r8188eu/include/hal_intf.h    |   6 +-
 .../staging/r8188eu/include/odm_interface.h   |   6 +-
 .../staging/r8188eu/include/rtl8188e_hal.h    |   2 +-
 drivers/staging/r8188eu/include/rtw_debug.h   |  13 +
 drivers/staging/r8188eu/include/rtw_efuse.h   |   4 +-
 drivers/staging/r8188eu/include/rtw_io.h      |  18 +-
 drivers/staging/r8188eu/include/rtw_mp.h      |   2 -
 drivers/staging/r8188eu/os_dep/ioctl_linux.c  | 179 +++++--
 drivers/staging/r8188eu/os_dep/usb_intf.c     |   3 +-
 30 files changed, 1143 insertions(+), 389 deletions(-)

-- 
2.32.0