[PATCH 00/13] s390/cio: replace page allocator calls with k[mz]alloc()

[PATCH 00/13] s390/cio: replace page allocator calls with k[mz]alloc()

[Mike Rapoport (Microsoft)]

The saga continues :)

This series covers CIO, QDIO, SCM and IDAL buffer allocations.

This is a (small) part of larger work of replacing page allocator calls
with kmalloc.

My initial intention a few month ago was to remove ugly casts [1], but then
willy pointed out that Linus objected to something like this [2] and it
looks like more than a decade old technical debt.

Largely, anything that doesn't need struct page (or a memdesc in the
future) should just use kmalloc() or kvmalloc() to allocate memory.
kmalloc() guarantees alignment, physical contiguity and working
virt_to_phys() and beside nicer API that returns void * on alloc and
doesn't require to know the allocation size on free, kmalloc() provides
better debugging capabilities than page allocator.

Another thing is that touching these allocation sites gives the reviewers
opportunity to see if a PAGE_SIZE buffer is actually needed or maybe
another size is appropriate.

For larger allocations that don't need physically contiguous memory
kvmalloc() can be a better option that __get_free_pages() because under
memory pressure it's is easier to allocate several order-0 pages than a
physically contiguous chunk with the same number of pages.

And last, but not least, removing needless calls to page allocator should
help with memdesc (aka project folio) conversion. There will be way less
places to audit to see if the user was actually using struct page.

The patches are grouped by buffer usage and ordered by file or
subsystem. Related CHSC work areas and requests are kept together.

While on it, use __free(kfree) for the SCM information buffer and local
buffers in synchronous CHSC ioctl handlers. The CHSC cleanup is kept in
a separate patch.

Also in git:
https://git.kernel.org/pub/scm/linux/kernel/git/rppt/linux.git gfp-to-kmalloc/s390-cio

[1] https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/
[2] https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/

---
Mike Rapoport (Microsoft) (13):
      s390/chsc: Use kzalloc() for CUBs
      s390/chsc: Use kzalloc() for the SEI work area
      s390/chsc_sch: Use kzalloc() for CHSC requests
      s390/chsc_sch: Use __free(kfree) for synchronous CHSC requests
      s390/cio: Use kzalloc() for CHSC work areas
      s390/cmf: Use kmalloc() for the CMB area
      s390/idals: Use kmalloc() for IDAL data buffers
      s390/qdio_main: Use kzalloc() for the IRQ structure
      s390/qdio_main: Use kzalloc() for the QDR
      s390/qdio_setup: Use kzalloc() for QDIO buffers
      s390/qdio_setup: Use kzalloc() for the storage list
      s390/qdio_setup: Use kzalloc() for the SSQD request
      s390/scm: Use kmalloc() for SCM information

 arch/s390/include/asm/idals.h   |   6 +-
 drivers/s390/cio/chsc.c         |  20 +--
 drivers/s390/cio/chsc_sch.c     | 300 ++++++++++++++--------------------------
 drivers/s390/cio/cmf.c          |   8 +-
 drivers/s390/cio/qdio.h         |   2 +-
 drivers/s390/cio/qdio_main.c    |  19 +--
 drivers/s390/cio/qdio_setup.c   |  14 +-
 drivers/s390/cio/qdio_thinint.c |   2 +-
 drivers/s390/cio/scm.c          |   7 +-
 9 files changed, 144 insertions(+), 234 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260907-s390-cio-ready-18e73ff8642f

--
Sincerely yours,
Mike.

[PATCH 01/13] s390/chsc: Use kzalloc() for CUBs

[Mike Rapoport (Microsoft)]

cub_alloc() allocates the channel measurement unit blocks and their
extended counterparts.

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/chsc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/s390/cio/chsc.c b/drivers/s390/cio/chsc.c
index 9689f722c863c..60a4a7e6086ce 100644
--- a/drivers/s390/cio/chsc.c
+++ b/drivers/s390/cio/chsc.c
@@ -931,12 +931,12 @@ static int cub_alloc(struct channel_subsystem *css)
 	int i;
 
 	for (i = 0; i < CSS_NUM_CUB_PAGES; i++) {
-		css->cub[i] = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+		css->cub[i] = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 		if (!css->cub[i])
 			return -ENOMEM;
 	}
 	for (i = 0; i < CSS_NUM_ECUB_PAGES; i++) {
-		css->ecub[i] = (void *)get_zeroed_page(GFP_KERNEL);
+		css->ecub[i] = kzalloc(PAGE_SIZE, GFP_KERNEL);
 		if (!css->ecub[i])
 			return -ENOMEM;
 	}
@@ -949,11 +949,11 @@ static void cub_free(struct channel_subsystem *css)
 	int i;
 
 	for (i = 0; i < CSS_NUM_CUB_PAGES; i++) {
-		free_page((unsigned long)css->cub[i]);
+		kfree(css->cub[i]);
 		css->cub[i] = NULL;
 	}
 	for (i = 0; i < CSS_NUM_ECUB_PAGES; i++) {
-		free_page((unsigned long)css->ecub[i]);
+		kfree(css->ecub[i]);
 		css->ecub[i] = NULL;
 	}
 }

-- 
2.53.0

Re: [PATCH 01/13] s390/chsc: Use kzalloc() for CUBs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] Hardware Alignment Violation

Pre-existing issues:
- [High] DMA Use-After-Free
--

commit c0b30acb33e9033572c3388a78f4e8cd3bf3f838
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

s390/chsc: Use kzalloc() for CUBs

cub_alloc() allocates the channel measurement unit blocks and their
extended counterparts.

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/s390/cio/chsc.c b/drivers/s390/cio/chsc.c
> index 9689f722c863c..60a4a7e6086ce 100644
> --- a/drivers/s390/cio/chsc.c
> +++ b/drivers/s390/cio/chsc.c
> @@ -931,12 +931,12 @@ static int cub_alloc(struct channel_subsystem *css)
>  	int i;
>  
>  	for (i = 0; i < CSS_NUM_CUB_PAGES; i++) {
> -		css->cub[i] = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
> +		css->cub[i] = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);

[Severity: Critical]
Does replacing get_zeroed_page() with kzalloc() break hardware alignment
requirements?

The original get_zeroed_page() allocated directly from the page allocator,
which strictly guarantees 4KB page alignment. kzalloc(PAGE_SIZE) allocates
from the generic slab allocator, which does not guarantee page alignment
when slab debugging (SLUB_DEBUG) or KASAN is enabled due to redzones and
padding.

When these pointers are passed to the hardware:

drivers/s390/cio/chsc.c:__chsc_do_secm() {
    ...
    for (i = 0; i < CSS_NUM_CUB_PAGES; i++)
        secm_area->cub[i] = (__force dma64_t)virt_to_dma32(css->cub[i]);
    ...
}

The s390 Store Extended Channel Measurement (SECM) hardware expects the
measurement unit blocks to be strictly aligned to a 4KB boundary. If the
pointer from kzalloc() is not aligned, the hardware will either reject the
instruction or mask off the lower 12 bits, writing DMA data to the start
of the slab page instead of the object offset. This can silently overwrite
redzones or preceding objects in the slab.

Can we continue to use a page-aligned allocator for these DMA blocks?

>  		if (!css->cub[i])
>  			return -ENOMEM;
>  	}
[ ... ]
> @@ -949,11 +949,11 @@ static void cub_free(struct channel_subsystem *css)
>  	int i;
>  
>  	for (i = 0; i < CSS_NUM_CUB_PAGES; i++) {
> -		free_page((unsigned long)css->cub[i]);
> +		kfree(css->cub[i]);
>  		css->cub[i] = NULL;
>  	}
>  	for (i = 0; i < CSS_NUM_ECUB_PAGES; i++) {
> -		free_page((unsigned long)css->ecub[i]);
> +		kfree(css->ecub[i]);
>  		css->ecub[i] = NULL;
>  	}
>  }

[Severity: High]
This isn't a bug introduced by this patch, but can this error path lead to a
DMA use-after-free?

In the rollback path of chsc_secm() which eventually calls cub_free():

drivers/s390/cio/chsc.c:chsc_secm() {
    ...
    if (!ret) {
        css->cm_enabled = enable;
        if (css->cm_enabled) {
            ret = chsc_add_cmg_attr(css);
            if (ret) {
                __chsc_do_secm(css, 0);
                css->cm_enabled = 0;
            }
    ...
out:
    if (!css->cm_enabled)
        cub_free(css);

If __chsc_do_secm(css, 0) fails (for example, returning -EBUSY because the
channel subsystem is busy), the hardware ignores the stop command and
continues performing DMA writes to the allocated memory blocks.

However, the code ignores the return value, unconditionally clears
css->cm_enabled, and frees the DMA memory via cub_free(). The hardware will
then continue to update the measurement blocks in the freed memory.

Should this path handle the failure of __chsc_do_secm() before freeing the
memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=1

Re: [PATCH 01/13] s390/chsc: Use kzalloc() for CUBs

[Mike Rapoport]

On Mon, Sep 07, 2026 at 10:37:18AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Critical] Hardware Alignment Violation
> 
> Pre-existing issues:
> - [High] DMA Use-After-Free
> --
> 
> commit c0b30acb33e9033572c3388a78f4e8cd3bf3f838
> Author: Mike Rapoport (Microsoft) <rppt@kernel.org>
> 
> s390/chsc: Use kzalloc() for CUBs
> 
> cub_alloc() allocates the channel measurement unit blocks and their
> extended counterparts.
> 
> This memory can be allocated with kmalloc() as there's nothing special
> about it to go directly to the page allocator.
> 
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> 
> > diff --git a/drivers/s390/cio/chsc.c b/drivers/s390/cio/chsc.c
> > index 9689f722c863c..60a4a7e6086ce 100644
> > --- a/drivers/s390/cio/chsc.c
> > +++ b/drivers/s390/cio/chsc.c
> > @@ -931,12 +931,12 @@ static int cub_alloc(struct channel_subsystem *css)
> >  	int i;
> >  
> >  	for (i = 0; i < CSS_NUM_CUB_PAGES; i++) {
> > -		css->cub[i] = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
> > +		css->cub[i] = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
> 
> [Severity: Critical]
> Does replacing get_zeroed_page() with kzalloc() break hardware alignment
> requirements?

No, kmalloc guarantees PAGE_SIZE alignment for PAGE_SIZE allocations.

I really need Chris to merge this :)
https://github.com/masoncl/review-prompts/pull/97
 
> The original get_zeroed_page() allocated directly from the page allocator,
> which strictly guarantees 4KB page alignment. kzalloc(PAGE_SIZE) allocates
> from the generic slab allocator, which does not guarantee page alignment
> when slab debugging (SLUB_DEBUG) or KASAN is enabled due to redzones and
> padding.
> 
> When these pointers are passed to the hardware:
> 
> drivers/s390/cio/chsc.c:__chsc_do_secm() {
>     ...
>     for (i = 0; i < CSS_NUM_CUB_PAGES; i++)
>         secm_area->cub[i] = (__force dma64_t)virt_to_dma32(css->cub[i]);
>     ...
> }
> 
> The s390 Store Extended Channel Measurement (SECM) hardware expects the
> measurement unit blocks to be strictly aligned to a 4KB boundary. If the
> pointer from kzalloc() is not aligned, the hardware will either reject the
> instruction or mask off the lower 12 bits, writing DMA data to the start
> of the slab page instead of the object offset. This can silently overwrite
> redzones or preceding objects in the slab.
> 
> Can we continue to use a page-aligned allocator for these DMA blocks?
> 
> >  		if (!css->cub[i])
> >  			return -ENOMEM;
> >  	}
> [ ... ]
> > @@ -949,11 +949,11 @@ static void cub_free(struct channel_subsystem *css)
> >  	int i;
> >  
> >  	for (i = 0; i < CSS_NUM_CUB_PAGES; i++) {
> > -		free_page((unsigned long)css->cub[i]);
> > +		kfree(css->cub[i]);
> >  		css->cub[i] = NULL;
> >  	}
> >  	for (i = 0; i < CSS_NUM_ECUB_PAGES; i++) {
> > -		free_page((unsigned long)css->ecub[i]);
> > +		kfree(css->ecub[i]);
> >  		css->ecub[i] = NULL;
> >  	}
> >  }
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but can this error path lead to a
> DMA use-after-free?
> 
> In the rollback path of chsc_secm() which eventually calls cub_free():
> 
> drivers/s390/cio/chsc.c:chsc_secm() {
>     ...
>     if (!ret) {
>         css->cm_enabled = enable;
>         if (css->cm_enabled) {
>             ret = chsc_add_cmg_attr(css);
>             if (ret) {
>                 __chsc_do_secm(css, 0);
>                 css->cm_enabled = 0;
>             }
>     ...
> out:
>     if (!css->cm_enabled)
>         cub_free(css);
> 
> If __chsc_do_secm(css, 0) fails (for example, returning -EBUSY because the
> channel subsystem is busy), the hardware ignores the stop command and
> continues performing DMA writes to the allocated memory blocks.
> 
> However, the code ignores the return value, unconditionally clears
> css->cm_enabled, and frees the DMA memory via cub_free(). The hardware will
> then continue to update the measurement blocks in the freed memory.
> 
> Should this path handle the failure of __chsc_do_secm() before freeing the
> memory?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=1

-- 
Sincerely yours,
Mike.

[PATCH 02/13] s390/chsc: Use kzalloc() for the SEI work area

[Mike Rapoport (Microsoft)]

chsc_init() allocates the work area for store event information data.

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/chsc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/cio/chsc.c b/drivers/s390/cio/chsc.c
index 60a4a7e6086ce..c3186c0a372b4 100644
--- a/drivers/s390/cio/chsc.c
+++ b/drivers/s390/cio/chsc.c
@@ -1142,7 +1142,7 @@ int __init chsc_init(void)
 {
 	int ret;
 
-	sei_page = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	sei_page = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	chsc_page = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
 	if (!sei_page || !chsc_page) {
 		ret = -ENOMEM;
@@ -1154,7 +1154,7 @@ int __init chsc_init(void)
 	return ret;
 out_err:
 	free_page((unsigned long)chsc_page);
-	free_page((unsigned long)sei_page);
+	kfree(sei_page);
 	return ret;
 }
 
@@ -1162,7 +1162,7 @@ void __init chsc_init_cleanup(void)
 {
 	crw_unregister_handler(CRW_RSC_CSS);
 	free_page((unsigned long)chsc_page);
-	free_page((unsigned long)sei_page);
+	kfree(sei_page);
 }
 
 int __chsc_enable_facility(struct chsc_sda_area *sda_area, int operation_code)

-- 
2.53.0

Re: [PATCH 02/13] s390/chsc: Use kzalloc() for the SEI work area

[sashiko-bot]

> chsc_init() allocates the work area for store event information data.
> 
> This memory can be allocated with kmalloc() as there's nothing special
> about it to go directly to the page allocator.
> 
> kmalloc() provides a better API that does not require ugly casts and
> kfree() does not need to know the size of the freed object.
> 
> Performance difference between kmalloc() and __get_free_pages() is not
> measurable as both allocators take an object/page from a per-CPU list for
> fast path allocations.
> 
> For the slow path the performance is anyway determined by the amount of
> reclaim involved rather than by what allocator is used.
> 
> Replace use of get_zeroed_page() with kzalloc() and free_page() with
> kfree().
> 
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> Assisted-by: copilot:claude-opus
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=2

[PATCH 03/13] s390/chsc_sch: Use kzalloc() for CHSC requests

[Mike Rapoport (Microsoft)]

The CHSC ioctl handlers allocate request and response areas for
CHSC commands issued on behalf of userspace.

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/chsc_sch.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/s390/cio/chsc_sch.c b/drivers/s390/cio/chsc_sch.c
index b6cb8bb8bcc4b..c8d6871d1841e 100644
--- a/drivers/s390/cio/chsc_sch.c
+++ b/drivers/s390/cio/chsc_sch.c
@@ -292,7 +292,7 @@ static int chsc_ioctl_start(void __user *user_area)
 	if (!css_general_characteristics.dynio)
 		/* It makes no sense to try. */
 		return -EOPNOTSUPP;
-	chsc_area = (void *)get_zeroed_page(GFP_DMA | GFP_KERNEL);
+	chsc_area = kzalloc(PAGE_SIZE, GFP_DMA | GFP_KERNEL);
 	if (!chsc_area)
 		return -ENOMEM;
 	request = kzalloc_obj(*request);
@@ -321,7 +321,7 @@ static int chsc_ioctl_start(void __user *user_area)
 	snprintf(dbf, sizeof(dbf), "ret:%d", ret);
 	CHSC_LOG(0, dbf);
 	kfree(request);
-	free_page((unsigned long)chsc_area);
+	kfree(chsc_area);
 	return ret;
 }
 
@@ -340,7 +340,7 @@ static int chsc_ioctl_on_close_set(void __user *user_area)
 		ret = -ENOMEM;
 		goto out_unlock;
 	}
-	on_close_chsc_area = (void *)get_zeroed_page(GFP_DMA | GFP_KERNEL);
+	on_close_chsc_area = kzalloc(PAGE_SIZE, GFP_DMA | GFP_KERNEL);
 	if (!on_close_chsc_area) {
 		ret = -ENOMEM;
 		goto out_free_request;
@@ -353,7 +353,7 @@ static int chsc_ioctl_on_close_set(void __user *user_area)
 	goto out_unlock;
 
 out_free_chsc:
-	free_page((unsigned long)on_close_chsc_area);
+	kfree(on_close_chsc_area);
 	on_close_chsc_area = NULL;
 out_free_request:
 	kfree(on_close_request);
@@ -375,7 +375,7 @@ static int chsc_ioctl_on_close_remove(void)
 		ret = -ENOENT;
 		goto out_unlock;
 	}
-	free_page((unsigned long)on_close_chsc_area);
+	kfree(on_close_chsc_area);
 	on_close_chsc_area = NULL;
 	kfree(on_close_request);
 	on_close_request = NULL;
@@ -392,7 +392,7 @@ static int chsc_ioctl_start_sync(void __user *user_area)
 	struct chsc_sync_area *chsc_area;
 	int ret, ccode;
 
-	chsc_area = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	chsc_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!chsc_area)
 		return -ENOMEM;
 	if (copy_from_user(chsc_area, user_area, PAGE_SIZE)) {
@@ -414,7 +414,7 @@ static int chsc_ioctl_start_sync(void __user *user_area)
 	else
 		ret = 0;
 out_free:
-	free_page((unsigned long)chsc_area);
+	kfree(chsc_area);
 	return ret;
 }
 
@@ -438,7 +438,7 @@ static int chsc_ioctl_info_channel_path(void __user *user_cd)
 		u8 data[PAGE_SIZE - 20];
 	} __attribute__ ((packed)) *scpcd_area;
 
-	scpcd_area = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	scpcd_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!scpcd_area)
 		return -ENOMEM;
 	cd = kzalloc_obj(*cd);
@@ -476,7 +476,7 @@ static int chsc_ioctl_info_channel_path(void __user *user_cd)
 		ret = 0;
 out_free:
 	kfree(cd);
-	free_page((unsigned long)scpcd_area);
+	kfree(scpcd_area);
 	return ret;
 }
 
@@ -500,7 +500,7 @@ static int chsc_ioctl_info_cu(void __user *user_cd)
 		u8 data[PAGE_SIZE - 20];
 	} __attribute__ ((packed)) *scucd_area;
 
-	scucd_area = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	scucd_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!scucd_area)
 		return -ENOMEM;
 	cd = kzalloc_obj(*cd);
@@ -538,7 +538,7 @@ static int chsc_ioctl_info_cu(void __user *user_cd)
 		ret = 0;
 out_free:
 	kfree(cd);
-	free_page((unsigned long)scucd_area);
+	kfree(scucd_area);
 	return ret;
 }
 
@@ -563,7 +563,7 @@ static int chsc_ioctl_info_sch_cu(void __user *user_cud)
 		u8 data[PAGE_SIZE - 20];
 	} __attribute__ ((packed)) *sscud_area;
 
-	sscud_area = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	sscud_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!sscud_area)
 		return -ENOMEM;
 	cud = kzalloc_obj(*cud);
@@ -602,7 +602,7 @@ static int chsc_ioctl_info_sch_cu(void __user *user_cud)
 		ret = 0;
 out_free:
 	kfree(cud);
-	free_page((unsigned long)sscud_area);
+	kfree(sscud_area);
 	return ret;
 }
 
@@ -625,7 +625,7 @@ static int chsc_ioctl_conf_info(void __user *user_ci)
 		u8 data[PAGE_SIZE - 20];
 	} __attribute__ ((packed)) *sci_area;
 
-	sci_area = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	sci_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!sci_area)
 		return -ENOMEM;
 	ci = kzalloc_obj(*ci);
@@ -662,7 +662,7 @@ static int chsc_ioctl_conf_info(void __user *user_ci)
 		ret = 0;
 out_free:
 	kfree(ci);
-	free_page((unsigned long)sci_area);
+	kfree(sci_area);
 	return ret;
 }
 
@@ -696,7 +696,7 @@ static int chsc_ioctl_conf_comp_list(void __user *user_ccl)
 		u32 res;
 	} __attribute__ ((packed)) *cssids_parm;
 
-	sccl_area = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	sccl_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!sccl_area)
 		return -ENOMEM;
 	ccl = kzalloc_obj(*ccl);
@@ -745,7 +745,7 @@ static int chsc_ioctl_conf_comp_list(void __user *user_ccl)
 		ret = 0;
 out_free:
 	kfree(ccl);
-	free_page((unsigned long)sccl_area);
+	kfree(sccl_area);
 	return ret;
 }
 
@@ -756,7 +756,7 @@ static int chsc_ioctl_chpd(void __user *user_chpd)
 	int ret;
 
 	chpd = kzalloc_obj(*chpd);
-	scpd_area = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	scpd_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!scpd_area || !chpd) {
 		ret = -ENOMEM;
 		goto out_free;
@@ -775,7 +775,7 @@ static int chsc_ioctl_chpd(void __user *user_chpd)
 		ret = -EFAULT;
 out_free:
 	kfree(chpd);
-	free_page((unsigned long)scpd_area);
+	kfree(scpd_area);
 	return ret;
 }
 
@@ -796,7 +796,7 @@ static int chsc_ioctl_dcal(void __user *user_dcal)
 		u8 data[PAGE_SIZE - 36];
 	} __attribute__ ((packed)) *sdcal_area;
 
-	sdcal_area = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	sdcal_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!sdcal_area)
 		return -ENOMEM;
 	dcal = kzalloc_obj(*dcal);
@@ -834,7 +834,7 @@ static int chsc_ioctl_dcal(void __user *user_dcal)
 		ret = 0;
 out_free:
 	kfree(dcal);
-	free_page((unsigned long)sdcal_area);
+	kfree(sdcal_area);
 	return ret;
 }
 
@@ -904,7 +904,7 @@ static int chsc_release(struct inode *inode, struct file *filp)
 	}
 	snprintf(dbf, sizeof(dbf), "relret:%d", ret);
 	CHSC_LOG(0, dbf);
-	free_page((unsigned long)on_close_chsc_area);
+	kfree(on_close_chsc_area);
 	on_close_chsc_area = NULL;
 	kfree(on_close_request);
 	on_close_request = NULL;

-- 
2.53.0

Re: [PATCH 03/13] s390/chsc_sch: Use kzalloc() for CHSC requests

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The device reference counting in the `chsc_async` loop is flawed, causing both a memory leak on the `continue` path and a Use-After-Free (UAF) on the normal loop continuation path.
--

commit 53c8650f3f5b3ba23ce15639dc05928d14ac4215
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

s390/chsc_sch: Use kzalloc() for CHSC requests

This commit replaces get_zeroed_page() with kzalloc() and free_page() with
kfree() for allocating CHSC requests. This simplifies the allocation and
free paths by using the standard slab allocator API.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/s390/cio/chsc_sch.c b/drivers/s390/cio/chsc_sch.c
> index b6cb8bb8bcc4b..c8d6871d1841e 100644
> --- a/drivers/s390/cio/chsc_sch.c
> +++ b/drivers/s390/cio/chsc_sch.c
> @@ -292,7 +292,7 @@ static int chsc_ioctl_start(void __user *user_area)
>  	if (!css_general_characteristics.dynio)
>  		/* It makes no sense to try. */
>  		return -EOPNOTSUPP;
> -	chsc_area = (void *)get_zeroed_page(GFP_DMA | GFP_KERNEL);
> +	chsc_area = kzalloc(PAGE_SIZE, GFP_DMA | GFP_KERNEL);
>  	if (!chsc_area)
>  		return -ENOMEM;
>  	request = kzalloc_obj(*request);

[Severity: High]
This is a pre-existing issue, but does the call to chsc_async() from within
this function trigger a memory leak and a use-after-free?

When the CHSC_START ioctl is called, it triggers chsc_async(). In that
function, the subchannel reference counting appears flawed:

drivers/s390/cio/chsc_sch.c:chsc_async() {
    ...
    while ((sch = chsc_get_next_subchannel(sch))) {
        ...
        if (private->request) {
            spin_unlock(&sch->lock);
            ret = -EBUSY;
            continue;
        }
        ...
        put_device(&sch->dev);
    }
    ...
}

If the chosen subchannel has private->request set, the loop hits continue,
which seems to skip dropping the subchannel reference and leaks it.

If the loop continues normally, it calls put_device(&sch->dev) and loops,
passing the now-put (and potentially freed) sch pointer back into
chsc_get_next_subchannel(). Does this subsequently dereference sch->dev.p
in driver_find_device(), resulting in a use-after-free?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=3

[PATCH 04/13] s390/chsc_sch: Use __free(kfree) for synchronous CHSC requests

[Mike Rapoport (Microsoft)]

Use __free(kfree) for local buffers in the synchronous CHSC ioctl
handlers and replace their cleanup labels with early returns.

Keep the asynchronous and on-close handlers unchanged.

Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/chsc_sch.c | 272 +++++++++++++++-----------------------------
 1 file changed, 92 insertions(+), 180 deletions(-)

diff --git a/drivers/s390/cio/chsc_sch.c b/drivers/s390/cio/chsc_sch.c
index c8d6871d1841e..225aaac7dd309 100644
--- a/drivers/s390/cio/chsc_sch.c
+++ b/drivers/s390/cio/chsc_sch.c
@@ -15,6 +15,7 @@
 #include <linux/uaccess.h>
 #include <linux/miscdevice.h>
 #include <linux/kernel_stat.h>
+#include <linux/cleanup.h>
 
 #include <asm/cio.h>
 #include <asm/chsc.h>
@@ -389,39 +390,29 @@ static int chsc_ioctl_on_close_remove(void)
 
 static int chsc_ioctl_start_sync(void __user *user_area)
 {
-	struct chsc_sync_area *chsc_area;
-	int ret, ccode;
+	struct chsc_sync_area *chsc_area __free(kfree) = NULL;
+	int ccode;
 
 	chsc_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!chsc_area)
 		return -ENOMEM;
-	if (copy_from_user(chsc_area, user_area, PAGE_SIZE)) {
-		ret = -EFAULT;
-		goto out_free;
-	}
-	if (chsc_area->header.code & 0x4000) {
-		ret = -EINVAL;
-		goto out_free;
-	}
+	if (copy_from_user(chsc_area, user_area, PAGE_SIZE))
+		return -EFAULT;
+	if (chsc_area->header.code & 0x4000)
+		return -EINVAL;
 	chsc_log_command(chsc_area);
 	ccode = chsc(chsc_area);
-	if (ccode != 0) {
-		ret = -EIO;
-		goto out_free;
-	}
+	if (ccode != 0)
+		return -EIO;
 	if (copy_to_user(user_area, chsc_area, PAGE_SIZE))
-		ret = -EFAULT;
-	else
-		ret = 0;
-out_free:
-	kfree(chsc_area);
-	return ret;
+		return -EFAULT;
+	return 0;
 }
 
 static int chsc_ioctl_info_channel_path(void __user *user_cd)
 {
-	struct chsc_chp_cd *cd;
-	int ret, ccode;
+	struct chsc_chp_cd *cd __free(kfree) = NULL;
+	int ccode;
 	struct {
 		struct chsc_header request;
 		u32 : 2;
@@ -436,20 +427,16 @@ static int chsc_ioctl_info_channel_path(void __user *user_cd)
 		u32 : 32;
 		struct chsc_header response;
 		u8 data[PAGE_SIZE - 20];
-	} __attribute__ ((packed)) *scpcd_area;
+	} __attribute__ ((packed)) *scpcd_area __free(kfree) = NULL;
 
 	scpcd_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!scpcd_area)
 		return -ENOMEM;
 	cd = kzalloc_obj(*cd);
-	if (!cd) {
-		ret = -ENOMEM;
-		goto out_free;
-	}
-	if (copy_from_user(cd, user_cd, sizeof(*cd))) {
-		ret = -EFAULT;
-		goto out_free;
-	}
+	if (!cd)
+		return -ENOMEM;
+	if (copy_from_user(cd, user_cd, sizeof(*cd)))
+		return -EFAULT;
 	scpcd_area->request.length = 0x0010;
 	scpcd_area->request.code = 0x0028;
 	scpcd_area->m = cd->m;
@@ -459,31 +446,23 @@ static int chsc_ioctl_info_channel_path(void __user *user_cd)
 	scpcd_area->last_chpid = cd->chpid.id;
 
 	ccode = chsc(scpcd_area);
-	if (ccode != 0) {
-		ret = -EIO;
-		goto out_free;
-	}
+	if (ccode != 0)
+		return -EIO;
 	if (scpcd_area->response.code != 0x0001) {
-		ret = -EIO;
 		CHSC_MSG(0, "scpcd: response code=%x\n",
 			 scpcd_area->response.code);
-		goto out_free;
+		return -EIO;
 	}
 	memcpy(&cd->cpcb, &scpcd_area->response, scpcd_area->response.length);
 	if (copy_to_user(user_cd, cd, sizeof(*cd)))
-		ret = -EFAULT;
-	else
-		ret = 0;
-out_free:
-	kfree(cd);
-	kfree(scpcd_area);
-	return ret;
+		return -EFAULT;
+	return 0;
 }
 
 static int chsc_ioctl_info_cu(void __user *user_cd)
 {
-	struct chsc_cu_cd *cd;
-	int ret, ccode;
+	struct chsc_cu_cd *cd __free(kfree) = NULL;
+	int ccode;
 	struct {
 		struct chsc_header request;
 		u32 : 2;
@@ -498,20 +477,16 @@ static int chsc_ioctl_info_cu(void __user *user_cd)
 		u32 : 32;
 		struct chsc_header response;
 		u8 data[PAGE_SIZE - 20];
-	} __attribute__ ((packed)) *scucd_area;
+	} __attribute__ ((packed)) *scucd_area __free(kfree) = NULL;
 
 	scucd_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!scucd_area)
 		return -ENOMEM;
 	cd = kzalloc_obj(*cd);
-	if (!cd) {
-		ret = -ENOMEM;
-		goto out_free;
-	}
-	if (copy_from_user(cd, user_cd, sizeof(*cd))) {
-		ret = -EFAULT;
-		goto out_free;
-	}
+	if (!cd)
+		return -ENOMEM;
+	if (copy_from_user(cd, user_cd, sizeof(*cd)))
+		return -EFAULT;
 	scucd_area->request.length = 0x0010;
 	scucd_area->request.code = 0x0026;
 	scucd_area->m = cd->m;
@@ -521,31 +496,23 @@ static int chsc_ioctl_info_cu(void __user *user_cd)
 	scucd_area->last_cun = cd->cun;
 
 	ccode = chsc(scucd_area);
-	if (ccode != 0) {
-		ret = -EIO;
-		goto out_free;
-	}
+	if (ccode != 0)
+		return -EIO;
 	if (scucd_area->response.code != 0x0001) {
-		ret = -EIO;
 		CHSC_MSG(0, "scucd: response code=%x\n",
 			 scucd_area->response.code);
-		goto out_free;
+		return -EIO;
 	}
 	memcpy(&cd->cucb, &scucd_area->response, scucd_area->response.length);
 	if (copy_to_user(user_cd, cd, sizeof(*cd)))
-		ret = -EFAULT;
-	else
-		ret = 0;
-out_free:
-	kfree(cd);
-	kfree(scucd_area);
-	return ret;
+		return -EFAULT;
+	return 0;
 }
 
 static int chsc_ioctl_info_sch_cu(void __user *user_cud)
 {
-	struct chsc_sch_cud *cud;
-	int ret, ccode;
+	struct chsc_sch_cud *cud __free(kfree) = NULL;
+	int ccode;
 	struct {
 		struct chsc_header request;
 		u32 : 2;
@@ -561,20 +528,16 @@ static int chsc_ioctl_info_sch_cu(void __user *user_cud)
 		u32 : 32;
 		struct chsc_header response;
 		u8 data[PAGE_SIZE - 20];
-	} __attribute__ ((packed)) *sscud_area;
+	} __attribute__ ((packed)) *sscud_area __free(kfree) = NULL;
 
 	sscud_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!sscud_area)
 		return -ENOMEM;
 	cud = kzalloc_obj(*cud);
-	if (!cud) {
-		ret = -ENOMEM;
-		goto out_free;
-	}
-	if (copy_from_user(cud, user_cud, sizeof(*cud))) {
-		ret = -EFAULT;
-		goto out_free;
-	}
+	if (!cud)
+		return -ENOMEM;
+	if (copy_from_user(cud, user_cud, sizeof(*cud)))
+		return -EFAULT;
 	sscud_area->request.length = 0x0010;
 	sscud_area->request.code = 0x0006;
 	sscud_area->m = cud->schid.m;
@@ -585,31 +548,23 @@ static int chsc_ioctl_info_sch_cu(void __user *user_cud)
 	sscud_area->last_sch = cud->schid.sch_no;
 
 	ccode = chsc(sscud_area);
-	if (ccode != 0) {
-		ret = -EIO;
-		goto out_free;
-	}
+	if (ccode != 0)
+		return -EIO;
 	if (sscud_area->response.code != 0x0001) {
-		ret = -EIO;
 		CHSC_MSG(0, "sscud: response code=%x\n",
 			 sscud_area->response.code);
-		goto out_free;
+		return -EIO;
 	}
 	memcpy(&cud->scub, &sscud_area->response, sscud_area->response.length);
 	if (copy_to_user(user_cud, cud, sizeof(*cud)))
-		ret = -EFAULT;
-	else
-		ret = 0;
-out_free:
-	kfree(cud);
-	kfree(sscud_area);
-	return ret;
+		return -EFAULT;
+	return 0;
 }
 
 static int chsc_ioctl_conf_info(void __user *user_ci)
 {
-	struct chsc_conf_info *ci;
-	int ret, ccode;
+	struct chsc_conf_info *ci __free(kfree) = NULL;
+	int ccode;
 	struct {
 		struct chsc_header request;
 		u32 : 2;
@@ -623,20 +578,16 @@ static int chsc_ioctl_conf_info(void __user *user_ci)
 		u64 : 64;
 		struct chsc_header response;
 		u8 data[PAGE_SIZE - 20];
-	} __attribute__ ((packed)) *sci_area;
+	} __attribute__ ((packed)) *sci_area __free(kfree) = NULL;
 
 	sci_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!sci_area)
 		return -ENOMEM;
 	ci = kzalloc_obj(*ci);
-	if (!ci) {
-		ret = -ENOMEM;
-		goto out_free;
-	}
-	if (copy_from_user(ci, user_ci, sizeof(*ci))) {
-		ret = -EFAULT;
-		goto out_free;
-	}
+	if (!ci)
+		return -ENOMEM;
+	if (copy_from_user(ci, user_ci, sizeof(*ci)))
+		return -EFAULT;
 	sci_area->request.length = 0x0010;
 	sci_area->request.code = 0x0012;
 	sci_area->m = ci->id.m;
@@ -645,31 +596,23 @@ static int chsc_ioctl_conf_info(void __user *user_ci)
 	sci_area->ssid = ci->id.ssid;
 
 	ccode = chsc(sci_area);
-	if (ccode != 0) {
-		ret = -EIO;
-		goto out_free;
-	}
+	if (ccode != 0)
+		return -EIO;
 	if (sci_area->response.code != 0x0001) {
-		ret = -EIO;
 		CHSC_MSG(0, "sci: response code=%x\n",
 			 sci_area->response.code);
-		goto out_free;
+		return -EIO;
 	}
 	memcpy(&ci->scid, &sci_area->response, sci_area->response.length);
 	if (copy_to_user(user_ci, ci, sizeof(*ci)))
-		ret = -EFAULT;
-	else
-		ret = 0;
-out_free:
-	kfree(ci);
-	kfree(sci_area);
-	return ret;
+		return -EFAULT;
+	return 0;
 }
 
 static int chsc_ioctl_conf_comp_list(void __user *user_ccl)
 {
-	struct chsc_comp_list *ccl;
-	int ret, ccode;
+	struct chsc_comp_list *ccl __free(kfree) = NULL;
+	int ccode;
 	struct {
 		struct chsc_header request;
 		u32 ctype : 8;
@@ -681,7 +624,7 @@ static int chsc_ioctl_conf_comp_list(void __user *user_ccl)
 		u64 : 64;
 		struct chsc_header response;
 		u8 data[PAGE_SIZE - 36];
-	} __attribute__ ((packed)) *sccl_area;
+	} __attribute__ ((packed)) *sccl_area __free(kfree) = NULL;
 	struct {
 		u32 m : 1;
 		u32 : 31;
@@ -700,14 +643,10 @@ static int chsc_ioctl_conf_comp_list(void __user *user_ccl)
 	if (!sccl_area)
 		return -ENOMEM;
 	ccl = kzalloc_obj(*ccl);
-	if (!ccl) {
-		ret = -ENOMEM;
-		goto out_free;
-	}
-	if (copy_from_user(ccl, user_ccl, sizeof(*ccl))) {
-		ret = -EFAULT;
-		goto out_free;
-	}
+	if (!ccl)
+		return -ENOMEM;
+	if (copy_from_user(ccl, user_ccl, sizeof(*ccl)))
+		return -EFAULT;
 	sccl_area->request.length = 0x0020;
 	sccl_area->request.code = 0x0030;
 	sccl_area->fmt = ccl->req.fmt;
@@ -728,61 +667,46 @@ static int chsc_ioctl_conf_comp_list(void __user *user_ccl)
 		break;
 	}
 	ccode = chsc(sccl_area);
-	if (ccode != 0) {
-		ret = -EIO;
-		goto out_free;
-	}
+	if (ccode != 0)
+		return -EIO;
 	if (sccl_area->response.code != 0x0001) {
-		ret = -EIO;
 		CHSC_MSG(0, "sccl: response code=%x\n",
 			 sccl_area->response.code);
-		goto out_free;
+		return -EIO;
 	}
 	memcpy(&ccl->sccl, &sccl_area->response, sccl_area->response.length);
 	if (copy_to_user(user_ccl, ccl, sizeof(*ccl)))
-		ret = -EFAULT;
-	else
-		ret = 0;
-out_free:
-	kfree(ccl);
-	kfree(sccl_area);
-	return ret;
+		return -EFAULT;
+	return 0;
 }
 
 static int chsc_ioctl_chpd(void __user *user_chpd)
 {
-	struct chsc_scpd *scpd_area;
-	struct chsc_cpd_info *chpd;
+	struct chsc_scpd *scpd_area __free(kfree) = NULL;
+	struct chsc_cpd_info *chpd __free(kfree) = NULL;
 	int ret;
 
 	chpd = kzalloc_obj(*chpd);
 	scpd_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
-	if (!scpd_area || !chpd) {
-		ret = -ENOMEM;
-		goto out_free;
-	}
-	if (copy_from_user(chpd, user_chpd, sizeof(*chpd))) {
-		ret = -EFAULT;
-		goto out_free;
-	}
+	if (!scpd_area || !chpd)
+		return -ENOMEM;
+	if (copy_from_user(chpd, user_chpd, sizeof(*chpd)))
+		return -EFAULT;
 	ret = chsc_determine_channel_path_desc(chpd->chpid, chpd->fmt,
 					       chpd->rfmt, chpd->c, chpd->m,
 					       scpd_area);
 	if (ret)
-		goto out_free;
+		return ret;
 	memcpy(&chpd->chpdb, &scpd_area->response, scpd_area->response.length);
 	if (copy_to_user(user_chpd, chpd, sizeof(*chpd)))
-		ret = -EFAULT;
-out_free:
-	kfree(chpd);
-	kfree(scpd_area);
-	return ret;
+		return -EFAULT;
+	return 0;
 }
 
 static int chsc_ioctl_dcal(void __user *user_dcal)
 {
-	struct chsc_dcal *dcal;
-	int ret, ccode;
+	struct chsc_dcal *dcal __free(kfree) = NULL;
+	int ccode;
 	struct {
 		struct chsc_header request;
 		u32 atype : 8;
@@ -794,20 +718,16 @@ static int chsc_ioctl_dcal(void __user *user_dcal)
 		u32 res1[2];
 		struct chsc_header response;
 		u8 data[PAGE_SIZE - 36];
-	} __attribute__ ((packed)) *sdcal_area;
+	} __attribute__ ((packed)) *sdcal_area __free(kfree) = NULL;
 
 	sdcal_area = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!sdcal_area)
 		return -ENOMEM;
 	dcal = kzalloc_obj(*dcal);
-	if (!dcal) {
-		ret = -ENOMEM;
-		goto out_free;
-	}
-	if (copy_from_user(dcal, user_dcal, sizeof(*dcal))) {
-		ret = -EFAULT;
-		goto out_free;
-	}
+	if (!dcal)
+		return -ENOMEM;
+	if (copy_from_user(dcal, user_dcal, sizeof(*dcal)))
+		return -EFAULT;
 	sdcal_area->request.length = 0x0020;
 	sdcal_area->request.code = 0x0034;
 	sdcal_area->atype = dcal->req.atype;
@@ -816,26 +736,18 @@ static int chsc_ioctl_dcal(void __user *user_dcal)
 	       sizeof(sdcal_area->list_parm));
 
 	ccode = chsc(sdcal_area);
-	if (ccode != 0) {
-		ret = -EIO;
-		goto out_free;
-	}
+	if (ccode != 0)
+		return -EIO;
 	if (sdcal_area->response.code != 0x0001) {
-		ret = -EIO;
 		CHSC_MSG(0, "sdcal: response code=%x\n",
 			 sdcal_area->response.code);
-		goto out_free;
+		return -EIO;
 	}
 	memcpy(&dcal->sdcal, &sdcal_area->response,
 	       sdcal_area->response.length);
 	if (copy_to_user(user_dcal, dcal, sizeof(*dcal)))
-		ret = -EFAULT;
-	else
-		ret = 0;
-out_free:
-	kfree(dcal);
-	kfree(sdcal_area);
-	return ret;
+		return -EFAULT;
+	return 0;
 }
 
 static long chsc_ioctl(struct file *filp, unsigned int cmd,

-- 
2.53.0

Re: [PATCH 04/13] s390/chsc_sch: Use __free(kfree) for synchronous CHSC requests

[sashiko-bot]

> Use __free(kfree) for local buffers in the synchronous CHSC ioctl
> handlers and replace their cleanup labels with early returns.
> 
> Keep the asynchronous and on-close handlers unchanged.
> 
> Assisted-by: copilot:claude-opus
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=4

[PATCH 05/13] s390/cio: Use kzalloc() for CHSC work areas

[Mike Rapoport (Microsoft)]

chsc_init() allocates the work area for CHSC commands and
qdio_allocate() allocates one for CHSC calls during qdio_establish().

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

While on it, change qdio_irq.chsc_page to void * to get rid of the casts.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/chsc.c         | 6 +++---
 drivers/s390/cio/qdio.h         | 2 +-
 drivers/s390/cio/qdio_main.c    | 6 +++---
 drivers/s390/cio/qdio_setup.c   | 2 +-
 drivers/s390/cio/qdio_thinint.c | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/s390/cio/chsc.c b/drivers/s390/cio/chsc.c
index c3186c0a372b4..d98f629e89295 100644
--- a/drivers/s390/cio/chsc.c
+++ b/drivers/s390/cio/chsc.c
@@ -1143,7 +1143,7 @@ int __init chsc_init(void)
 	int ret;
 
 	sei_page = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
-	chsc_page = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	chsc_page = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!sei_page || !chsc_page) {
 		ret = -ENOMEM;
 		goto out_err;
@@ -1153,7 +1153,7 @@ int __init chsc_init(void)
 		goto out_err;
 	return ret;
 out_err:
-	free_page((unsigned long)chsc_page);
+	kfree(chsc_page);
 	kfree(sei_page);
 	return ret;
 }
@@ -1161,7 +1161,7 @@ int __init chsc_init(void)
 void __init chsc_init_cleanup(void)
 {
 	crw_unregister_handler(CRW_RSC_CSS);
-	free_page((unsigned long)chsc_page);
+	kfree(chsc_page);
 	kfree(sei_page);
 }
 
diff --git a/drivers/s390/cio/qdio.h b/drivers/s390/cio/qdio.h
index 4bd4c00c9c0ca..dff5f53a8795d 100644
--- a/drivers/s390/cio/qdio.h
+++ b/drivers/s390/cio/qdio.h
@@ -244,7 +244,7 @@ struct qdio_irq {
 	int perf_stat_enabled;
 
 	struct qdr *qdr;
-	unsigned long chsc_page;
+	void *chsc_page;
 
 	struct qdio_q *input_qs[QDIO_MAX_QUEUES_PER_IRQ];
 	struct qdio_q *output_qs[QDIO_MAX_QUEUES_PER_IRQ];
diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
index c1e09fa34e774..d137bf8c70664 100644
--- a/drivers/s390/cio/qdio_main.c
+++ b/drivers/s390/cio/qdio_main.c
@@ -934,7 +934,7 @@ int qdio_free(struct ccw_device *cdev)
 
 	qdio_free_queues(irq_ptr);
 	free_page((unsigned long) irq_ptr->qdr);
-	free_page(irq_ptr->chsc_page);
+	kfree(irq_ptr->chsc_page);
 	kfree(irq_ptr->ccw);
 	free_page((unsigned long) irq_ptr);
 	return 0;
@@ -986,7 +986,7 @@ int qdio_allocate(struct ccw_device *cdev, unsigned int no_input_qs,
 	 * qdio_establish. In case of low memory and swap on a zfcp disk
 	 * we may not be able to allocate memory otherwise.
 	 */
-	irq_ptr->chsc_page = get_zeroed_page(GFP_KERNEL);
+	irq_ptr->chsc_page = kzalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!irq_ptr->chsc_page)
 		goto err_chsc;
 
@@ -1006,7 +1006,7 @@ int qdio_allocate(struct ccw_device *cdev, unsigned int no_input_qs,
 err_queues:
 	free_page((unsigned long) irq_ptr->qdr);
 err_qdr:
-	free_page(irq_ptr->chsc_page);
+	kfree(irq_ptr->chsc_page);
 err_chsc:
 err_dbf:
 	kfree(irq_ptr->ccw);
diff --git a/drivers/s390/cio/qdio_setup.c b/drivers/s390/cio/qdio_setup.c
index ea09aadaae4ec..bd80703c174c7 100644
--- a/drivers/s390/cio/qdio_setup.c
+++ b/drivers/s390/cio/qdio_setup.c
@@ -253,7 +253,7 @@ int qdio_setup_get_ssqd(struct qdio_irq *irq_ptr,
 		if (!ssqd)
 			return -ENOMEM;
 	} else {
-		ssqd = (struct chsc_ssqd_area *)irq_ptr->chsc_page;
+		ssqd = irq_ptr->chsc_page;
 	}
 
 	rc = chsc_ssqd(*schid, ssqd);
diff --git a/drivers/s390/cio/qdio_thinint.c b/drivers/s390/cio/qdio_thinint.c
index e167aa75c3dff..a2ff51887537f 100644
--- a/drivers/s390/cio/qdio_thinint.c
+++ b/drivers/s390/cio/qdio_thinint.c
@@ -136,7 +136,7 @@ static struct airq_struct tiqdio_airq = {
 
 static int set_subchannel_ind(struct qdio_irq *irq_ptr, int reset)
 {
-	struct chsc_scssc_area *scssc = (void *)irq_ptr->chsc_page;
+	struct chsc_scssc_area *scssc = irq_ptr->chsc_page;
 	dma64_t summary_indicator_addr, subchannel_indicator_addr;
 	int rc;
 

-- 
2.53.0

Re: [PATCH 05/13] s390/cio: Use kzalloc() for CHSC work areas

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing get_zeroed_page() with kzalloc() for CHSC (Channel Subsystem Call) memory blocks breaks the strict 4KB hardware alignment requirement when SLUB debugging or KASAN is enabled.
--

commit 9694c5915181f336b62527d191701841da8a5e9a
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

s390/cio: Use kzalloc() for CHSC work areas

chsc_init() allocates the work area for chsc commands and
qdio_allocate() allocates one for chsc calls during qdio_establish().
This commit replaces the use of get_zeroed_page() with kzalloc() and
free_page() with kfree() for these allocations.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/s390/cio/chsc.c b/drivers/s390/cio/chsc.c
> index c3186c0a372b4..d98f629e89295 100644
> --- a/drivers/s390/cio/chsc.c
> +++ b/drivers/s390/cio/chsc.c
> @@ -1143,7 +1143,7 @@ int __init chsc_init(void)
>  	int ret;
>  
>  	sei_page = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
> -	chsc_page = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
> +	chsc_page = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);

[Severity: High]
Does replacing get_zeroed_page() with kzalloc() cause a regression by
breaking the strict 4KB hardware alignment requirement for chsc command
blocks?

While kzalloc(PAGE_SIZE) might return page-aligned memory by default, if
slub debugging (like redzoning) or kasan is enabled, metadata is inserted
before the object, shifting the returned pointer away from the page boundary.
Passing this unaligned pointer to the chsc instruction could cause the
hardware to raise a specification exception and crash the system.

[ ... ]

> diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
> index c1e09fa34e774..d137bf8c70664 100644
> --- a/drivers/s390/cio/qdio_main.c
> +++ b/drivers/s390/cio/qdio_main.c
> @@ -986,7 +986,7 @@ int qdio_allocate(struct ccw_device *cdev, unsigned int no_input_qs,
>  	 * qdio_establish. In case of low memory and swap on a zfcp disk
>  	 * we may not be able to allocate memory otherwise.
>  	 */
> -	irq_ptr->chsc_page = get_zeroed_page(GFP_KERNEL);
> +	irq_ptr->chsc_page = kzalloc(PAGE_SIZE, GFP_KERNEL);

[Severity: High]
Could this allocation introduce a similar regression with 4KB hardware
alignment?

Since the qdio chsc page is also used for hardware instructions in
qdio_setup_get_ssqd() and set_subchannel_ind(), losing the strict page
alignment guarantees of get_zeroed_page() when debugging is enabled might
lead to unaligned command blocks and hardware specification exceptions.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=5

[PATCH 06/13] s390/cmf: Use kmalloc() for the CMB area

[Mike Rapoport (Microsoft)]

alloc_cmb() allocates the channel measurement block area shared by
devices using the basic channel measurement format.

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

The measurement block origin must be page aligned and kmalloc()
guarantees that a power of two sized allocation is aligned to its size.

Replace use of __get_free_pages() with kmalloc() and free_pages() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/cmf.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/s390/cio/cmf.c b/drivers/s390/cio/cmf.c
index 92ab3d546fe47..6c46b0d0b3da4 100644
--- a/drivers/s390/cio/cmf.c
+++ b/drivers/s390/cio/cmf.c
@@ -501,12 +501,12 @@ static int alloc_cmb(struct ccw_device *cdev)
 		WARN_ON(!list_empty(&cmb_area.list));
 
 		spin_unlock(&cmb_area.lock);
-		mem = (void *)__get_free_pages(GFP_KERNEL, get_order(size));
+		mem = kmalloc(PAGE_SIZE << get_order(size), GFP_KERNEL);
 		spin_lock(&cmb_area.lock);
 
 		if (cmb_area.mem) {
 			/* ok, another thread was faster */
-			free_pages((unsigned long)mem, get_order(size));
+			kfree(mem);
 		} else if (!mem) {
 			/* no luck */
 			ret = -ENOMEM;
@@ -547,10 +547,8 @@ static void free_cmb(struct ccw_device *cdev)
 	list_del_init(&priv->cmb_list);
 
 	if (list_empty(&cmb_area.list)) {
-		ssize_t size;
-		size = sizeof(struct cmb) * cmb_area.num_channels;
 		cmf_activate(NULL, CMF_OFF);
-		free_pages((unsigned long)cmb_area.mem, get_order(size));
+		kfree(cmb_area.mem);
 		cmb_area.mem = NULL;
 	}
 	spin_unlock_irq(cdev->ccwlock);

-- 
2.53.0

Re: [PATCH 06/13] s390/cmf: Use kmalloc() for the CMB area

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Integer overflow to zero-size allocation causing lowcore memory corruption.
--

commit 2913241109f97628467377f13982dfc657e8d846
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

s390/cmf: Use kmalloc() for the CMB area

This commit replaces __get_free_pages() with kmalloc() for allocating
the channel measurement block area, and updates the free path to use
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/s390/cio/cmf.c b/drivers/s390/cio/cmf.c
> index 92ab3d546fe47..6c46b0d0b3da4 100644
> --- a/drivers/s390/cio/cmf.c
> +++ b/drivers/s390/cio/cmf.c
> @@ -501,12 +501,12 @@ static int alloc_cmb(struct ccw_device *cdev)
>  		WARN_ON(!list_empty(&cmb_area.list));
>  
>  		spin_unlock(&cmb_area.lock);
> -		mem = (void *)__get_free_pages(GFP_KERNEL, get_order(size));
> +		mem = kmalloc(PAGE_SIZE << get_order(size), GFP_KERNEL);

[Severity: Critical]
If size evaluates to 0 (for example, if the maxchannels module parameter
is configured to 0 by the user), get_order(0) underflows and returns 52 on
64-bit s390x.

The allocation size expression PAGE_SIZE << 52 evaluates to
(1UL << 12) << 52, which wraps around modulo 64 and evaluates to exactly 0.

Will kmalloc(0, GFP_KERNEL) then return the ZERO_SIZE_PTR sentinel value
(0x10) instead of NULL?

>  		spin_lock(&cmb_area.lock);
>  
>  		if (cmb_area.mem) {
>  			/* ok, another thread was faster */
> -			free_pages((unsigned long)mem, get_order(size));
> +			kfree(mem);
>  		} else if (!mem) {

[Severity: Critical]
Since ZERO_SIZE_PTR != NULL, will this error check fail to catch the
sentinel value?

If this check fails, the code continues to the success path:

drivers/s390/cio/cmf.c:alloc_cmb() {
...
		} else {
			/* everything ok */
			memset(mem, 0, size);
			cmb_area.mem = mem;
			cmf_activate(cmb_area.mem, CMF_ON);
		}
...
}

For size == 0, memset(ZERO_SIZE_PTR, 0, 0) does nothing. The code then
proceeds to call cmf_activate((void *)0x10, CMF_ON).

Since the schm hardware instruction is given physical address 0x10, will this
instruct the channel subsystem to blindly write DMA measurement data into the
CPU's lowcore (PSA), resulting in system crashes by destroying critical kernel
state?

Prior to this patch, __get_free_pages(GFP_KERNEL, 52) gracefully failed by
returning NULL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=6

Re: [PATCH 06/13] s390/cmf: Use kmalloc() for the CMB area

[Mike Rapoport]

On Mon, Sep 07, 2026 at 10:35:39AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Integer overflow to zero-size allocation causing lowcore memory corruption.
> --
> 
> commit 2913241109f97628467377f13982dfc657e8d846
> Author: Mike Rapoport (Microsoft) <rppt@kernel.org>
> 
> s390/cmf: Use kmalloc() for the CMB area
> 
> This commit replaces __get_free_pages() with kmalloc() for allocating
> the channel measurement block area, and updates the free path to use
> kfree().
> 
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> 
> > diff --git a/drivers/s390/cio/cmf.c b/drivers/s390/cio/cmf.c
> > index 92ab3d546fe47..6c46b0d0b3da4 100644
> > --- a/drivers/s390/cio/cmf.c
> > +++ b/drivers/s390/cio/cmf.c
> > @@ -501,12 +501,12 @@ static int alloc_cmb(struct ccw_device *cdev)
> >  		WARN_ON(!list_empty(&cmb_area.list));
> >  
> >  		spin_unlock(&cmb_area.lock);
> > -		mem = (void *)__get_free_pages(GFP_KERNEL, get_order(size));
> > +		mem = kmalloc(PAGE_SIZE << get_order(size), GFP_KERNEL);
> 
> [Severity: Critical]
> If size evaluates to 0 (for example, if the maxchannels module parameter
> is configured to 0 by the user), get_order(0) underflows and returns 52 on
> 64-bit s390x.

I missed that one while reviewing automated conversion. It should be

	mem = kmalloc(PAGE_ALIGN(size), GFP_KERNEL);
 
> The allocation size expression PAGE_SIZE << 52 evaluates to
> (1UL << 12) << 52, which wraps around modulo 64 and evaluates to exactly 0.

-- 
Sincerely yours,
Mike.

[PATCH 07/13] s390/idals: Use kmalloc() for IDAL data buffers

[Mike Rapoport (Microsoft)]

idal_buffer_alloc() allocates the data chunks of an IDAL buffer that is
used for channel I/O.

These buffers can be allocated with kmalloc() as there's nothing special
about them to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of __get_free_pages() with kmalloc() and free_pages() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/s390/include/asm/idals.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/s390/include/asm/idals.h b/arch/s390/include/asm/idals.h
index 06e1ec2afd5af..248829d461bce 100644
--- a/arch/s390/include/asm/idals.h
+++ b/arch/s390/include/asm/idals.h
@@ -147,7 +147,7 @@ static inline struct idal_buffer *idal_buffer_alloc(size_t size, int page_order)
 			ib->data[i] = dma64_add(ib->data[i - 1], IDA_BLOCK_SIZE);
 			continue;
 		}
-		vaddr = (void *)__get_free_pages(GFP_KERNEL, page_order);
+		vaddr = kmalloc(PAGE_SIZE << page_order, GFP_KERNEL);
 		if (!vaddr)
 			goto error;
 		ib->data[i] = virt_to_dma64(vaddr);
@@ -157,7 +157,7 @@ static inline struct idal_buffer *idal_buffer_alloc(size_t size, int page_order)
 	while (i >= nr_chunks) {
 		i -= nr_chunks;
 		vaddr = dma64_to_virt(ib->data[i]);
-		free_pages((unsigned long)vaddr, ib->page_order);
+		kfree(vaddr);
 	}
 	kfree(ib);
 	return ERR_PTR(-ENOMEM);
@@ -175,7 +175,7 @@ static inline void idal_buffer_free(struct idal_buffer *ib)
 	nr_chunks = (PAGE_SIZE << ib->page_order) >> IDA_SIZE_SHIFT;
 	for (i = 0; i < nr_ptrs; i += nr_chunks) {
 		vaddr = dma64_to_virt(ib->data[i]);
-		free_pages((unsigned long)vaddr, ib->page_order);
+		kfree(vaddr);
 	}
 	kfree(ib);
 }

-- 
2.53.0

Re: [PATCH 07/13] s390/idals: Use kmalloc() for IDAL data buffers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Replacing __get_free_pages() with kmalloc() violates the strict 4K page-alignment requirement for s390 Indirect Data Address Words (IDAWs), leading to hardware I/O rejection and out-of-bounds memory access.
- [Medium] Kmemleak will falsely report IDAL data chunks as memory leaks because only their physical addresses are stored.

Pre-existing issues:
- [High] Unprotected concurrent modification of device->char_data.ibs in tape_check_idalbuffer() leading to a Use-After-Free.
--

commit ee00e66345d4d1d6997d0f2f7bda11ae7ad10516
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

s390/idals: Use kmalloc() for IDAL data buffers

idal_buffer_alloc() allocates the data chunks of an IDAL buffer that is
used for channel I/O.
These buffers can be allocated with kmalloc() as there's nothing special
about them to go directly to the page allocator.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/arch/s390/include/asm/idals.h b/arch/s390/include/asm/idals.h
> index 06e1ec2afd5af..248829d461bce 100644
> --- a/arch/s390/include/asm/idals.h
> +++ b/arch/s390/include/asm/idals.h
> @@ -147,7 +147,7 @@ static inline struct idal_buffer *idal_buffer_alloc(size_t size, int page_order)
>  			ib->data[i] = dma64_add(ib->data[i - 1], IDA_BLOCK_SIZE);
>  			continue;
>  		}
> -		vaddr = (void *)__get_free_pages(GFP_KERNEL, page_order);
> +		vaddr = kmalloc(PAGE_SIZE << page_order, GFP_KERNEL);

[Severity: High]
Does replacing __get_free_pages() with kmalloc() violate the strict 4K
page-alignment requirement for s390 Indirect Data Address Words (IDAWs)?

kmalloc() with memory debugging enabled (like SLUB redzoning) can return
unaligned addresses for PAGE_SIZE allocations. Since the s390 Channel
Subsystem mandates that second and subsequent IDAWs must be aligned to a
4K boundary, could this unaligned address cause hardware I/O rejection?

Additionally, if the base address is unaligned, does adding 4K via
dma64_add() result in out-of-bounds array access, since the nr_ptrs
calculation mathematically assumes a 4K-aligned start address?

>  		if (!vaddr)
>  			goto error;
>  		ib->data[i] = virt_to_dma64(vaddr);

[Severity: Medium]
Will kmemleak falsely report these IDAL data chunks as memory leaks?

Since virt_to_dma64() stores the physical address, the kmemleak scanner
won't find the exact virtual address bit pattern returned by kmalloc()
during its garbage collection scan.

[ ... ]

This is a pre-existing issue, but I noticed a potential race condition in
the tape driver when checking IDAL buffers.

In drivers/s390/char/tape_char.c:tapechar_read():

    rc = tape_check_idalbuffer(device, block_size);

And in drivers/s390/char/tape_core.c:tape_check_idalbuffer():

    idal_buffer_array_free(&device->char_data.ibs);
    device->char_data.ibs = new;

[Severity: High]
If two threads call read() or write() concurrently without device->mutex
being held, could one thread free device->char_data.ibs while the other
is executing tape_do_io() or iterating over the old ibs array? Can this
lead to a use-after-free or data corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=7

[PATCH 08/13] s390/qdio_main: Use kzalloc() for the IRQ structure

[Mike Rapoport (Microsoft)]

qdio_allocate() allocates the QDIO irq structure.

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/qdio_main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
index d137bf8c70664..6b9442bae7ffd 100644
--- a/drivers/s390/cio/qdio_main.c
+++ b/drivers/s390/cio/qdio_main.c
@@ -17,6 +17,7 @@
 #include <linux/gfp.h>
 #include <linux/io.h>
 #include <linux/atomic.h>
+#include <linux/slab.h>
 #include <asm/debug.h>
 #include <asm/qdio.h>
 #include <asm/asm.h>
@@ -936,7 +937,7 @@ int qdio_free(struct ccw_device *cdev)
 	free_page((unsigned long) irq_ptr->qdr);
 	kfree(irq_ptr->chsc_page);
 	kfree(irq_ptr->ccw);
-	free_page((unsigned long) irq_ptr);
+	kfree(irq_ptr);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(qdio_free);
@@ -961,7 +962,7 @@ int qdio_allocate(struct ccw_device *cdev, unsigned int no_input_qs,
 	    no_output_qs > QDIO_MAX_QUEUES_PER_IRQ)
 		return -EINVAL;
 
-	irq_ptr = (void *) get_zeroed_page(GFP_KERNEL);
+	irq_ptr = kzalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!irq_ptr)
 		return -ENOMEM;
 
@@ -1011,7 +1012,7 @@ int qdio_allocate(struct ccw_device *cdev, unsigned int no_input_qs,
 err_dbf:
 	kfree(irq_ptr->ccw);
 err_ccw:
-	free_page((unsigned long) irq_ptr);
+	kfree(irq_ptr);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(qdio_allocate);

-- 
2.53.0

Re: [PATCH 08/13] s390/qdio_main: Use kzalloc() for the IRQ structure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Replacing get_zeroed_page() with kzalloc(PAGE_SIZE) drops strict page-alignment guarantees, violating the 256-byte hardware alignment requirement for the embedded struct qib when SLUB debugging is enabled.
- [Low] Stale comment regarding kmemleak scanning for irq_ptr.
--

commit ca3c961e09a135301359d791ffa1a2e2f24317b3
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

s390/qdio_main: Use kzalloc() for the IRQ structure

This commit replaces the use of get_zeroed_page() and free_page() for the
QDIO IRQ structure with kzalloc() and kfree(). It aims to improve API usage
by eliminating casts and the need to track object size on free.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
> index d137bf8c70664..6b9442bae7ffd 100644
> --- a/drivers/s390/cio/qdio_main.c
> +++ b/drivers/s390/cio/qdio_main.c
[ ... ]
> @@ -961,7 +962,7 @@ int qdio_allocate(struct ccw_device *cdev, unsigned int no_input_qs,
>  	    no_output_qs > QDIO_MAX_QUEUES_PER_IRQ)
>  		return -EINVAL;
>  
> -	irq_ptr = (void *) get_zeroed_page(GFP_KERNEL);
> +	irq_ptr = kzalloc(PAGE_SIZE, GFP_KERNEL);

[Severity: High]
Does this allocation still meet the strict alignment requirements for the
embedded Queue Information Block (QIB) structure?

The QDIO hardware strictly requires the physical address of the QIB to be
256-byte aligned. Since struct qib is the first member of struct qdio_irq, it
was safely page-aligned when using get_zeroed_page().

With kzalloc(PAGE_SIZE), if CONFIG_SLUB_DEBUG is enabled and redzoning is
active, objects in power-of-two slab caches (like kmalloc-4096) are prefixed
with redzones. This shifts the object's starting address away from the
natural page boundary, falling back to ARCH_KMALLOC_MINALIGN (8 bytes on
s390).

Could this cause the physical address of irq_ptr->qib to be unaligned and
lead to hardware I/O operations failing?

[Severity: Low]
This isn't a bug, but does this change make the kmemleak comment and
annotation further down in qdio_allocate() obsolete?

drivers/s390/cio/qdio_main.c:qdio_allocate() {
    ...
	/* kmemleak doesn't scan the page-allocated irq_ptr: */
	kmemleak_not_leak(irq_ptr->ccw);
    ...
}

Since irq_ptr is now allocated using kzalloc(), the slab allocator will
automatically track it for kmemleak, making the workaround comment incorrect
and the kmemleak_not_leak() call functionally unnecessary.

>  	if (!irq_ptr)
>  		return -ENOMEM;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=8

[PATCH 09/13] s390/qdio_main: Use kzalloc() for the QDR

[Mike Rapoport (Microsoft)]

qdio_allocate() allocates the queue description record (QDR).

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/qdio_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c
index 6b9442bae7ffd..bebc1250ebebf 100644
--- a/drivers/s390/cio/qdio_main.c
+++ b/drivers/s390/cio/qdio_main.c
@@ -934,7 +934,7 @@ int qdio_free(struct ccw_device *cdev)
 	mutex_unlock(&irq_ptr->setup_mutex);
 
 	qdio_free_queues(irq_ptr);
-	free_page((unsigned long) irq_ptr->qdr);
+	kfree(irq_ptr->qdr);
 	kfree(irq_ptr->chsc_page);
 	kfree(irq_ptr->ccw);
 	kfree(irq_ptr);
@@ -992,7 +992,7 @@ int qdio_allocate(struct ccw_device *cdev, unsigned int no_input_qs,
 		goto err_chsc;
 
 	/* qdr is used in ccw1.cda which is u32 */
-	irq_ptr->qdr = (struct qdr *) get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	irq_ptr->qdr = kzalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!irq_ptr->qdr)
 		goto err_qdr;
 
@@ -1005,7 +1005,7 @@ int qdio_allocate(struct ccw_device *cdev, unsigned int no_input_qs,
 	return 0;
 
 err_queues:
-	free_page((unsigned long) irq_ptr->qdr);
+	kfree(irq_ptr->qdr);
 err_qdr:
 	kfree(irq_ptr->chsc_page);
 err_chsc:

-- 
2.53.0

Re: [PATCH 09/13] s390/qdio_main: Use kzalloc() for the QDR

[sashiko-bot]

> qdio_allocate() allocates the queue description record (QDR).
> 
> This memory can be allocated with kmalloc() as there's nothing special
> about it to go directly to the page allocator.
> 
> kmalloc() provides a better API that does not require ugly casts and
> kfree() does not need to know the size of the freed object.
> 
> Performance difference between kmalloc() and __get_free_pages() is not
> measurable as both allocators take an object/page from a per-CPU list for
> fast path allocations.
> 
> For the slow path the performance is anyway determined by the amount of
> reclaim involved rather than by what allocator is used.
> 
> Replace use of get_zeroed_page() with kzalloc() and free_page() with
> kfree().
> 
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> Assisted-by: copilot:claude-opus
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=9

[PATCH 10/13] s390/qdio_setup: Use kzalloc() for QDIO buffers

[Mike Rapoport (Microsoft)]

qdio_alloc_buffers() allocates the QDIO buffers.

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/qdio_setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/cio/qdio_setup.c b/drivers/s390/cio/qdio_setup.c
index bd80703c174c7..b6f2ff202fe64 100644
--- a/drivers/s390/cio/qdio_setup.c
+++ b/drivers/s390/cio/qdio_setup.c
@@ -35,7 +35,7 @@ void qdio_free_buffers(struct qdio_buffer **buf, unsigned int count)
 	int pos;
 
 	for (pos = 0; pos < count; pos += QBUFF_PER_PAGE)
-		free_page((unsigned long) buf[pos]);
+		kfree(buf[pos]);
 }
 EXPORT_SYMBOL_GPL(qdio_free_buffers);
 
@@ -49,7 +49,7 @@ int qdio_alloc_buffers(struct qdio_buffer **buf, unsigned int count)
 	int pos;
 
 	for (pos = 0; pos < count; pos += QBUFF_PER_PAGE) {
-		buf[pos] = (void *) get_zeroed_page(GFP_KERNEL);
+		buf[pos] = kzalloc(PAGE_SIZE, GFP_KERNEL);
 		if (!buf[pos]) {
 			qdio_free_buffers(buf, count);
 			return -ENOMEM;

-- 
2.53.0

Re: [PATCH 10/13] s390/qdio_setup: Use kzalloc() for QDIO buffers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing get_zeroed_page() with kzalloc() breaks the strict 256-byte hardware alignment requirement for QDIO SBALs when memory debugging is enabled.
--

commit cef648048ee4dbd28b972a034767786968da0d5a
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

s390/qdio_setup: Use kzalloc() for QDIO buffers

This commit replaces the use of get_zeroed_page() and free_page() with
kzalloc() and kfree() for allocating and freeing QDIO buffers.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/s390/cio/qdio_setup.c b/drivers/s390/cio/qdio_setup.c
> --- a/drivers/s390/cio/qdio_setup.c
> +++ b/drivers/s390/cio/qdio_setup.c

[ ... ]

> @@ -49,7 +49,7 @@ int qdio_alloc_buffers(struct qdio_buffer **buf, unsigned int count)
>  	int pos;
>  
>  	for (pos = 0; pos < count; pos += QBUFF_PER_PAGE) {
> -		buf[pos] = (void *) get_zeroed_page(GFP_KERNEL);
> +		buf[pos] = kzalloc(PAGE_SIZE, GFP_KERNEL);

[Severity: High]
Does this kzalloc() change break the 256-byte hardware alignment requirement
for QDIO SBALs?

The s390 QDIO hardware requires struct qdio_buffer to be strictly 256-byte
aligned.

While get_zeroed_page() guarantees page alignment, using kzalloc() with
PAGE_SIZE will fall into the kmalloc-4096 cache. If a memory debugging
feature like CONFIG_SLUB_DEBUG is enabled, the SLUB allocator adds a left
redzone before the object.

On s390, this redzone shifts the payload address by ARCH_KMALLOC_MINALIGN
(8 bytes). This shifts the address so the returned pointer is no longer
256-byte aligned, which can cause device probe failures or system crashes
when the misaligned address is rejected by the channel subsystem.

>  		if (!buf[pos]) {
>  			qdio_free_buffers(buf, count);
>  			return -ENOMEM;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=10

[PATCH 11/13] s390/qdio_setup: Use kzalloc() for the storage list

[Mike Rapoport (Microsoft)]

__qdio_allocate_qs() allocates the storage list information block of a
queue.

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of __get_free_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/qdio_setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/cio/qdio_setup.c b/drivers/s390/cio/qdio_setup.c
index b6f2ff202fe64..7ca8f61a2387b 100644
--- a/drivers/s390/cio/qdio_setup.c
+++ b/drivers/s390/cio/qdio_setup.c
@@ -83,7 +83,7 @@ static void __qdio_free_queues(struct qdio_q **queues, unsigned int count)
 
 	for (i = 0; i < count; i++) {
 		q = queues[i];
-		free_page((unsigned long)q->sl_page);
+		kfree(q->sl_page);
 		kmem_cache_free(qdio_q_cache, q);
 	}
 }
@@ -109,7 +109,7 @@ static int __qdio_allocate_qs(struct qdio_q **irq_ptr_qs, int nr_queues)
 			return -ENOMEM;
 		}
 
-		q->sl_page = (void *)__get_free_page(GFP_KERNEL);
+		q->sl_page = kzalloc(PAGE_SIZE, GFP_KERNEL);
 		if (!q->sl_page) {
 			kmem_cache_free(qdio_q_cache, q);
 			__qdio_free_queues(irq_ptr_qs, i);

-- 
2.53.0

Re: [PATCH 11/13] s390/qdio_setup: Use kzalloc() for the storage list

[sashiko-bot]

> __qdio_allocate_qs() allocates the storage list information block of a
> queue.
> 
> This memory can be allocated with kmalloc() as there's nothing special
> about it to go directly to the page allocator.
> 
> kmalloc() provides a better API that does not require ugly casts and
> kfree() does not need to know the size of the freed object.
> 
> Performance difference between kmalloc() and __get_free_pages() is not
> measurable as both allocators take an object/page from a per-CPU list for
> fast path allocations.
> 
> For the slow path the performance is anyway determined by the amount of
> reclaim involved rather than by what allocator is used.
> 
> Replace use of __get_free_page() with kzalloc() and free_page() with
> kfree().
> 
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> Assisted-by: copilot:claude-opus
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=11

[PATCH 12/13] s390/qdio_setup: Use kzalloc() for the SSQD request

[Mike Rapoport (Microsoft)]

qdio_setup_get_ssqd() allocates the request block for the Store
Subchannel QDIO Data (SSQD) CHSC command.

This memory can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of __get_free_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/qdio_setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/cio/qdio_setup.c b/drivers/s390/cio/qdio_setup.c
index 7ca8f61a2387b..c0215a7d29a81 100644
--- a/drivers/s390/cio/qdio_setup.c
+++ b/drivers/s390/cio/qdio_setup.c
@@ -249,7 +249,7 @@ int qdio_setup_get_ssqd(struct qdio_irq *irq_ptr,
 
 	DBF_EVENT("getssqd:%4x", schid->sch_no);
 	if (!irq_ptr) {
-		ssqd = (struct chsc_ssqd_area *)__get_free_page(GFP_KERNEL);
+		ssqd = kzalloc(PAGE_SIZE, GFP_KERNEL);
 		if (!ssqd)
 			return -ENOMEM;
 	} else {
@@ -270,7 +270,7 @@ int qdio_setup_get_ssqd(struct qdio_irq *irq_ptr,
 
 out:
 	if (!irq_ptr)
-		free_page((unsigned long)ssqd);
+		kfree(ssqd);
 
 	return rc;
 }

-- 
2.53.0

Re: [PATCH 12/13] s390/qdio_setup: Use kzalloc() for the SSQD request

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware specification exception due to unaligned CHSC command block allocation.
--

commit e45e0f584b812bc3fd8eb8fb6e196dc061a1cf06
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

s390/qdio_setup: Use kzalloc() for the SSQD request

This commit replaces the use of __get_free_page() and free_page() with
kzalloc() and kfree() for allocating the SSQD request block in
qdio_setup_get_ssqd().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/s390/cio/qdio_setup.c b/drivers/s390/cio/qdio_setup.c
> index 7ca8f61a2387b..c0215a7d29a81 100644
> --- a/drivers/s390/cio/qdio_setup.c
> +++ b/drivers/s390/cio/qdio_setup.c
> @@ -249,7 +249,7 @@ int qdio_setup_get_ssqd(struct qdio_irq *irq_ptr,
>  
>  	DBF_EVENT("getssqd:%4x", schid->sch_no);
>  	if (!irq_ptr) {
> -		ssqd = (struct chsc_ssqd_area *)__get_free_page(GFP_KERNEL);
> +		ssqd = kzalloc(PAGE_SIZE, GFP_KERNEL);

[Severity: High]
Will this kzalloc() allocation guarantee the strict page alignment required by
the hardware?

Under SLUB debug configurations (like redzoning or poisoning), kzalloc() does
not guarantee page-aligned addresses. The previously used __get_free_page()
ensured this alignment.

When this unaligned memory block is later passed to the hardware CHSC
instruction via chsc_ssqd(), could it trigger a hardware specification
exception (PGM 0x06) and lead to a kernel panic during QDIO device setup?

>  		if (!ssqd)
>  			return -ENOMEM;
>  	} else {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=12

[PATCH 13/13] s390/scm: Use kmalloc() for SCM information

[Mike Rapoport (Microsoft)]

scm_update_information() allocates the response buffer for the CHSC
Store SCM Information command.

This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of __get_free_page() with kmalloc() and free_page() with
kfree().

While on it, use __free(kfree) for the local response buffer.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 drivers/s390/cio/scm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/s390/cio/scm.c b/drivers/s390/cio/scm.c
index 171212a6d2d9c..b79fe023ab8d1 100644
--- a/drivers/s390/cio/scm.c
+++ b/drivers/s390/cio/scm.c
@@ -13,6 +13,7 @@
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/err.h>
+#include <linux/cleanup.h>
 #include <asm/eadm.h>
 #include "chsc.h"
 
@@ -224,12 +225,12 @@ static int scm_add(struct chsc_scm_info *scm_info, size_t num)
 
 int scm_update_information(void)
 {
-	struct chsc_scm_info *scm_info;
+	struct chsc_scm_info *scm_info __free(kfree) = NULL;
 	u64 token = 0;
 	size_t num;
 	int ret;
 
-	scm_info = (void *)__get_free_page(GFP_KERNEL | GFP_DMA);
+	scm_info = kmalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!scm_info)
 		return -ENOMEM;
 
@@ -250,8 +251,6 @@ int scm_update_information(void)
 		token = scm_info->restok;
 	} while (token);
 
-	free_page((unsigned long)scm_info);
-
 	return ret;
 }
 

-- 
2.53.0

Re: [PATCH 13/13] s390/scm: Use kmalloc() for SCM information

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing `__get_free_page()` with `kmalloc(PAGE_SIZE)` breaks the strict 4K page-alignment requirement of the CHSC instruction when SLUB debugging is enabled.
--

commit ad79881d62e47e7d8ac373d9ac1ace6a762ae9a4
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

s390/scm: Use kmalloc() for SCM information

This commit replaces the allocation of the CHSC Store SCM Information command
response buffer from __get_free_page() to kmalloc(). It also updates the
local response buffer to use the __free(kfree) scope-based cleanup helper.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/s390/cio/scm.c b/drivers/s390/cio/scm.c
> index 171212a6d2d9c..b79fe023ab8d1 100644
> --- a/drivers/s390/cio/scm.c
> +++ b/drivers/s390/cio/scm.c
[ ... ]
> @@ -224,12 +225,12 @@ static int scm_add(struct chsc_scm_info *scm_info, size_t num)
>  
>  int scm_update_information(void)
>  {
> -	struct chsc_scm_info *scm_info;
> +	struct chsc_scm_info *scm_info __free(kfree) = NULL;
>  	u64 token = 0;
>  	size_t num;
>  	int ret;
>  
> -	scm_info = (void *)__get_free_page(GFP_KERNEL | GFP_DMA);
> +	scm_info = kmalloc(PAGE_SIZE, GFP_KERNEL | GFP_DMA);

[Severity: High]
Does this change break the 4K page-alignment requirement for the CHSC
instruction?

In drivers/s390/cio/scm.c:scm_update_information(), replacing the natively
page-aligned __get_free_page() with kmalloc(PAGE_SIZE) means the returned
pointer can be unaligned when SLUB debugging or KASAN is enabled, due to
padding added before the object.

This scm_info pointer is passed down and ultimately used by the CHSC
instruction in drivers/s390/cio/ioasm.c:chsc(). The s390 architecture
strictly mandates that CHSC command blocks be placed on a 4K-byte boundary.

Executing the instruction with an unaligned address triggers a hardware
Specification Exception kernel crash.

>  	if (!scm_info)
>  		return -ENOMEM;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-s390-cio-ready-v1-0-ca7f39806234@kernel.org?part=13