All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] random: reject negative RNDADDENTROPY sizes early
@ 2026-06-28 11:42 Jakub Stasiak
  2026-06-29  2:12 ` Theodore Tso
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Stasiak @ 2026-06-28 11:42 UTC (permalink / raw)
  To: Theodore Ts'o, Jason A. Donenfeld; +Cc: linux-kernel

For the RNDADDENTROPY request the len value (byte count) comes from
rand_pool_info.buf_size, which is a signed int, and is then passed to
import_ubuf(), where the len parameter is size_t.

A negative len value would become a large unsigned value inside
import_ubuf() where it would be capped to MAX_RW_COUNT
and then potentially rejected by

	if (unlikely(!access_ok(buf, len)))
		return -EFAULT;

before actually copying anything. If that call succeeds, random_ioctl()
still rejects the request because the number of bytes written does not
match the specified len:

		ret = write_pool_user(&iter);
		...
		if (unlikely(ret != len))
			return -EFAULT;

Reject negative len values at the ioctl boundary instead. This avoids
relying on the later import and write paths, and prevents an invalid
request from mixing user data into the pool at the clamped length
instead of the requested len before returning -EFAULT. Consider this
defense in depth.

This affects userspace in that a different (but more appropriate) errno
will be returned in this case.

Assisted-by: Codex:gpt-5.5
Signed-off-by: Jakub Stasiak <jakub@stasiak.at>
---

First time submitting anything to the kernel, apologies if anything's off.

 drivers/char/random.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index b4da1fb976c1..f81c47ab95ba 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1517,6 +1517,8 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
 			return -EINVAL;
 		if (get_user(len, p++))
 			return -EFAULT;
+		if (len < 0)
+			return -EINVAL;
 		ret = import_ubuf(ITER_SOURCE, p, len, &iter);
 		if (unlikely(ret))
 			return ret;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] random: reject negative RNDADDENTROPY sizes early
  2026-06-28 11:42 [PATCH] random: reject negative RNDADDENTROPY sizes early Jakub Stasiak
@ 2026-06-29  2:12 ` Theodore Tso
  2026-06-29  7:23   ` Jakub Stasiak
  0 siblings, 1 reply; 3+ messages in thread
From: Theodore Tso @ 2026-06-29  2:12 UTC (permalink / raw)
  To: Jakub Stasiak; +Cc: Jason A. Donenfeld, linux-kernel

On Sun, Jun 28, 2026 at 01:42:27PM -0500, Jakub Stasiak wrote:
> Reject negative len values at the ioctl boundary instead. This avoids
> relying on the later import and write paths, and prevents an invalid
> request from mixing user data into the pool at the clamped length
> instead of the requested len before returning -EFAULT. Consider this
> defense in depth.

The patch isn't wrong per se, but defense in depth against what?  This
ioctl() requires root privileges, and so it's not something where we
would need to worry about an attacker passing in invalid data to the
ioctl.  If you have CAP_SYS_ADMIN, there is far more mischief you
could do....

					- Ted

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] random: reject negative RNDADDENTROPY sizes early
  2026-06-29  2:12 ` Theodore Tso
@ 2026-06-29  7:23   ` Jakub Stasiak
  0 siblings, 0 replies; 3+ messages in thread
From: Jakub Stasiak @ 2026-06-29  7:23 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Jason A. Donenfeld, linux-kernel



> On 29 Jun 2026, at 04:12, Theodore Tso <tytso@mit.edu> wrote:
> 
> On Sun, Jun 28, 2026 at 01:42:27PM -0500, Jakub Stasiak wrote:
>> Reject negative len values at the ioctl boundary instead. This avoids
>> relying on the later import and write paths, and prevents an invalid
>> request from mixing user data into the pool at the clamped length
>> instead of the requested len before returning -EFAULT. Consider this
>> defense in depth.
> 
> The patch isn't wrong per se, but defense in depth against what?  This
> ioctl() requires root privileges, and so it's not something where we
> would need to worry about an attacker passing in invalid data to the
> ioctl.  If you have CAP_SYS_ADMIN, there is far more mischief you
> could do....
> 
> - Ted

Ah yes, totally fair; I think I used the wrong term there, what I
meant is more about defensive programming (earlier rejection to make
sure nothing weird happens in case of accidentally bad inputs) and not
about defending against intentional attacks by someone with root
privileges.

Jakub

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-29  7:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-28 11:42 [PATCH] random: reject negative RNDADDENTROPY sizes early Jakub Stasiak
2026-06-29  2:12 ` Theodore Tso
2026-06-29  7:23   ` Jakub Stasiak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.