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
