Linux s390 Architecture development
 help / color / mirror / Atom feed
From: Alexander Egorenkov <egorenar@linux.ibm.com>
To: oberpar@linux.ibm.com
Cc: gor@linux.ibm.com, hca@linux.ibm.com, agordeev@linux.ibm.com,
	borntraeger@linux.ibm.com,
	linux390-list@tuxmaker.boeblingen.de.ibm.com,
	linux-s390@vger.kernel.org
Subject: [PATCH v6 0/5] s390/sclp: Misc fixes
Date: Tue, 22 Sep 2026 08:42:58 +0200	[thread overview]
Message-ID: <20260922064303.524293-1-egorenar@linux.ibm.com> (raw)

This series consists of several fixes for the s390 SCLP driver.

* The first patch makes sclp_dispatch_evbufs() more safe while parsing event buffers
  contained in a received SCCB buffer.
* The second patch introduces the macro sclp_gds_for_each() to safely iterate over
  GDS {sub}vectors and serves to improve error handling in sclp_find_gds_{sub}vector()
  to prevent out-of-range memory read and potential infinite loops
  when a malformed event buffer is received from SCLP.
* The third patch reuses the macro sclp_gds_for_each() in SCLP TTY introduced in the first patch
  to replace manual and error-prone iteration over entries of a GDS {sub}vector to fix
  the same issues addressed in the first patch.
* The fourth patch fixes 2 potential illegal memory accesses when reading
  the value from a GDS subvector in SCLP event buffers sent by OCF.
* The fifth patch fixes race situations with in-flight callbacks and sclp_unregister() calls.

Changes since v5
----------------
- Added patch "s390/sclp: Make parsing of received event buffers more robust"
  - Reported by Sashiko
- Rework the patch "s390/sclp: Introduce macro sclp_gds_for_each()"
  - Explicit cast of start and end parameters to void* removing the need
    for the caller to use extra void* variables for start and end, and also
    protecting from mistakes when using non-void* variables when calculating n
- Rework the patch "s390/sclp_ocf: Fix computation of length of GDS values"
  - Improved commit description as suggested by Peter

Changes since v4
----------------
- Drop patch "s390/sclp: Drop volatile type class from SCLP state variables"
  - There are several doubts to it being correct in all situations
- Introduce the macro sclp_gds_for_each()
  - Reusable and safe iteration over GDS {sub}vectors
  - Make use of sclp_gds_for_each() in SCLP TTY
- Rework the patch "s390/sclp: Ensure no callback gets called after sclp_{un}register() returns"
  - Remove waiting for SCLP mask and reading states to become idle from sclp_register() on sclp_init_mask() failure
    - First, it is incorrect to sleep in sclp_register() which is called from atomic context
      - sclp_console_init() -> sclp_rw_init() -> sclp_register()
      - sclp_vt220_con_init() -> __sclp_vt220_init() -> sclp_register()
    - Second, it is redundant because no race situation can occur if sclp_init_mask() fails because
      in that case no events can be received from SCLP due to SCLP receive event mask update performed
      in sclp_init_mask() having failed
  - Add might_sleep() to sclp_unregister() to indicate that the function could potentially sleep
  - Adjust coding style of function sclp_unregister()
- Add "Fixes" tag where necessary

Changes since v3
----------------
- Rework the patch "s390/sclp: Ensure no callback gets called after sclp_{un}register() returns"
  - Shorten and reword the commit description
  - Replace wake_up_all_locked() with wake_up_all()
  - Replace sclp_init_state with sclp_mask_state in wait queue condition
  - Call wake_up_all() unconditionally
  - Add call to wake_up_call() in sclp_init_mask() after updating sclp_mask_state 

Changes since v2
----------------
- Add 2 new patches:
  - s390/sclp: Drop volatile type class from SCLP state variables
  - s390/sclp: Improve robustness of sclp_find_gds_{sub}vector()
- Rework the patch "s390/sclp: Ensure no callback gets called after sclp_unregister() returns"
  to implement Peter Oberparleiter's suggestion with a global wait queue and checking
  the state variables as its condition. It turns out the implementation with a single completion
  per struct sclp_register is inadequate because theoretically the callback state_change_fn() and receive_fn()
  could get invoked in parallel, however unlikely. Furthermore, the same race situation might happen
  with sclp_register() too. 

Changes since v1
----------------
- Drop redundant empty lines in sclp.c
- Make commit message more verbose for the fix in sclp.c

Alexander Egorenkov (5):
  s390/sclp: Make parsing of received event buffers more robust
  s390/sclp: Introduce macro sclp_gds_for_each()
  s390/sclp_tty: Make use of sclp_gds_for_each()
  s390/sclp_ocf: Fix computation of length of GDS values
  s390/sclp: Ensure no callback gets called after sclp_unregister()
    returns

 drivers/s390/char/sclp.c     | 33 +++++++++++++++++++++++----------
 drivers/s390/char/sclp.h     | 24 ++++++++++++++++--------
 drivers/s390/char/sclp_ocf.c |  4 ++--
 drivers/s390/char/sclp_tty.c | 24 +++++++++---------------
 4 files changed, 50 insertions(+), 35 deletions(-)

-- 
2.53.0


             reply	other threads:[~2026-09-22  6:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-22  6:42 Alexander Egorenkov [this message]
2026-09-22  6:42 ` [PATCH v6 1/5] s390/sclp: Make parsing of received event buffers more robust Alexander Egorenkov
2026-09-22  6:51   ` sashiko-bot
2026-09-22  6:43 ` [PATCH v6 2/5] s390/sclp: Introduce macro sclp_gds_for_each() Alexander Egorenkov
2026-09-22  6:54   ` sashiko-bot
2026-09-22  6:43 ` [PATCH v6 3/5] s390/sclp_tty: Make use of sclp_gds_for_each() Alexander Egorenkov
2026-09-22  6:52   ` sashiko-bot
2026-09-22  6:43 ` [PATCH v6 4/5] s390/sclp_ocf: Fix computation of length of GDS values Alexander Egorenkov
2026-09-22  6:54   ` sashiko-bot
2026-09-22  6:43 ` [PATCH v6 5/5] s390/sclp: Ensure no callback gets called after sclp_unregister() returns Alexander Egorenkov
2026-09-22  6:52   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260922064303.524293-1-egorenar@linux.ibm.com \
    --to=egorenar@linux.ibm.com \
    --cc=agordeev@linux.ibm.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux390-list@tuxmaker.boeblingen.de.ibm.com \
    --cc=oberpar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox