Messages in this thread | | | Date | Wed, 30 Aug 2023 16:48:00 -0700 | From | Kees Cook <> | Subject | Re: [PATCH] vt: Fix potential read overflow of kernel memory |
| |
On Wed, Aug 30, 2023 at 05:17:12PM -0600, Dan Raymond wrote: > In my opinion strlcpy() is being used correctly here as a defensive > precaution. If the source string is larger than the destination buffer > it will truncate rather than corrupt kernel memory. However the > return value of strlcpy() is being misused. If truncation occurred > the copy_to_user() call will corrupt user memory instead. > > I also agree that this is not currently a bug. It is fragile and it > could break if someone added a very large string to the table. > > Why not fix this by avoiding the redundant string copy? How about > something like this: > > ptr = func_table[kb_func] ? : ""; > len = strlen(ptr); > > if (len >= sizeof(user_kdgkb->kb_string)) > return -ENOSPC; > > if (copy_to_user(user_kdgkb->kb_string, ptr, len + 1)) > return -EFAULT;
This would work if not for func_buf_lock. The bounce buffer is used to avoid needing to hold the spin lock across copy_to_user.
-- Kees Cook
| |