[PATCH net] s390/qeth: validate user buffer length in SNMP and ARP query ioctls

[PATCH net] s390/qeth: validate user buffer length in SNMP and ARP query ioctls

[Hidayath Khan]

qeth_snmp_command() and qeth_l3_arp_query() allocate a buffer sized by
a user-supplied length (udata_len) without checking a lower bound, then
set udata_offset to a fixed non-zero value and pass both to a reply
callback. The callback bounds-checks the copy with

        if ((udata_len - udata_offset) < len)

Both fields are u32, so a udata_len smaller than udata_offset makes the
subtraction wrap and the check pass, and the following memcpy() writes
past the allocation. A udata_len of 0 also yields ZERO_SIZE_PTR from
kzalloc(), which the existing NULL check does not catch.

Reject buffers smaller than udata_offset before allocating, so the
callback subtraction can no longer underflow.

Fixes: 4a71df50047f ("qeth: new qeth device driver")
Cc: stable@vger.kernel.org
Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
---
 drivers/s390/net/qeth_core_main.c | 3 +++
 drivers/s390/net/qeth_l3_main.c   | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index f18eed9df3c7..c3257b213360 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4710,6 +4710,9 @@ static int qeth_snmp_command(struct qeth_card *card, char __user *udata)
 	if (req_len > QETH_BUFSIZE)
 		return -EINVAL;
 
+	if (qinfo.udata_len < sizeof(struct qeth_snmp_ureq_hdr))
+		return -EINVAL;
+
 	iob = qeth_get_adapter_cmd(card, IPA_SETADP_SET_SNMP_CONTROL, req_len);
 	if (!iob)
 		return -ENOMEM;
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 1542bfc9f561..f1ac9950dcb4 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1415,6 +1415,11 @@ static int qeth_l3_arp_query(struct qeth_card *card, char __user *udata)
 		rc = -EFAULT;
 		goto out;
 	}
+
+	if (qinfo.udata_len < QETH_QARP_ENTRIES_OFFSET) {
+		rc = -EINVAL;
+		goto out;
+	}
 	qinfo.udata = kzalloc(qinfo.udata_len, GFP_KERNEL);
 	if (!qinfo.udata) {
 		rc = -ENOMEM;

base-commit: 58c1c294d685baf94801839334df20501711eb8e
-- 
2.52.0

Re: [PATCH net] s390/qeth: validate user buffer length in SNMP and ARP query ioctls

[Joe Damato]

On Thu, Jul 30, 2026 at 04:22:16PM +0200, Hidayath Khan wrote:
> qeth_snmp_command() and qeth_l3_arp_query() allocate a buffer sized by
> a user-supplied length (udata_len) without checking a lower bound, then
> set udata_offset to a fixed non-zero value and pass both to a reply
> callback. The callback bounds-checks the copy with
> 
>         if ((udata_len - udata_offset) < len)
> 
> Both fields are u32, so a udata_len smaller than udata_offset makes the
> subtraction wrap and the check pass, and the following memcpy() writes
> past the allocation. A udata_len of 0 also yields ZERO_SIZE_PTR from
> kzalloc(), which the existing NULL check does not catch.
> 
> Reject buffers smaller than udata_offset before allocating, so the
> callback subtraction can no longer underflow.
> 
> Fixes: 4a71df50047f ("qeth: new qeth device driver")
> Cc: stable@vger.kernel.org
> Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
> Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
> ---
>  drivers/s390/net/qeth_core_main.c | 3 +++
>  drivers/s390/net/qeth_l3_main.c   | 5 +++++
>  2 files changed, 8 insertions(+)

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net] s390/qeth: validate user buffer length in SNMP and ARP query ioctls

[sashiko-bot]

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

Pre-existing issues:
- [High] Unvalidated length fields from hardware responses lead to kernel heap out-of-bounds reads and memory disclosure.
--

--- Patch [1]: [PATCH net] s390/qeth: validate user buffer length in SNMP and ARP query ioctls ---
commit f9f8c7e047077e9dbc90a35127afc75e1690d27f
Author: Hidayath Khan <hidayath@linux.ibm.com>

s390/qeth: validate user buffer length in SNMP and ARP query ioctls

This commit validates the user-supplied length (udata_len) against a lower
bound (udata_offset) before allocating the buffer in SNMP and ARP query ioctls.
This prevents underflow during callback bounds checking and fixes a
ZERO_SIZE_PTR allocation when udata_len is 0.

> diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
> index f18eed9df3c7a..c3257b213360c 100644
> --- a/drivers/s390/net/qeth_core_main.c
> +++ b/drivers/s390/net/qeth_core_main.c
> @@ -4710,6 +4710,9 @@ static int qeth_snmp_command(struct qeth_card *card, char __user *udata)
>  	if (req_len > QETH_BUFSIZE)
>  		return -EINVAL;
>  
> +	if (qinfo.udata_len < sizeof(struct qeth_snmp_ureq_hdr))
> +		return -EINVAL;
> +

[Severity: High]
This is a pre-existing issue, but does this expose the kernel to a heap
out-of-bounds read if the hardware returns an invalid length?

In qeth_snmp_command_cb(), the response length is calculated from the
firmware's header:

	data_len = adp_cmd->hdr.cmdlength - sizeof(adp_cmd->hdr);
    ...
	if ((qinfo->udata_len - qinfo->udata_offset) < data_len) {
		return -ENOSPC;
	}
    ...
	memcpy(qinfo->udata + qinfo->udata_offset, snmp_data, data_len);

The check ensures data_len fits within the userspace buffer (qinfo->udata_len),
but doesn't verify if data_len exceeds the actual hardware response buffer
length. If a compromised or malicious firmware returns a large cmdlength (and
userspace provided a large udata_len), could this memcpy() read past the end of
the hardware response buffer and leak adjacent kernel memory back to userspace?

>  	iob = qeth_get_adapter_cmd(card, IPA_SETADP_SET_SNMP_CONTROL, req_len);
>  	if (!iob)
>  		return -ENOMEM;
> diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
> index 1542bfc9f561b..f1ac9950dcb4e 100644
> --- a/drivers/s390/net/qeth_l3_main.c
> +++ b/drivers/s390/net/qeth_l3_main.c
> @@ -1415,6 +1415,11 @@ static int qeth_l3_arp_query(struct qeth_card *card, char __user *udata)
>  		rc = -EFAULT;
>  		goto out;
>  	}
> +
> +	if (qinfo.udata_len < QETH_QARP_ENTRIES_OFFSET) {
> +		rc = -EINVAL;
> +		goto out;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but is there a similar out-of-bounds
read issue when processing the ARP entries?

In qeth_l3_arp_query_cb(), the code trusts the firmware-provided
qdata->no_entries without checking if the entries actually fit within the
received hardware response buffer:

	for (e = 0; e < qdata->no_entries; ++e) {
        ...
		esize = get_arp_entry_size(card, qdata, etype, do_strip_entries);
        ...
		if ((qinfo->udata_len - qinfo->udata_offset) < esize) {
			return -ENOSPC;
		}

		memcpy(qinfo->udata + qinfo->udata_offset,
			&qdata->data + entrybytes_done + stripped_bytes,
			esize);
		entrybytes_done += esize + stripped_bytes;

Like the SNMP issue above, the bounds check only verifies that there is enough
space in the destination user buffer (qinfo->udata_len), but does not check if
we are reading past the bounds of the hardware response buffer. Could a
malicious firmware response with a large no_entries cause a kernel heap
out-of-bounds read?

>  	qinfo.udata = kzalloc(qinfo.udata_len, GFP_KERNEL);
>  	if (!qinfo.udata) {
>  		rc = -ENOMEM;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730142216.218309-1-hidayath@linux.ibm.com?part=1