PRINTF_BUFR_SIZE dangerous?

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

PRINTF_BUFR_SIZE dangerous?

Gary Jennejohn-6
It seems like PRINTF_BUFR_SIZE is a kernel fault waiting to happen.

Only /usr/src/sys/cam/cam_xpt.c asserts that it's <= a maximum value of
512 bytes.

/usr/src/sys/kern/tty.c uses it to malloc space without checking its size.

/usr/src/sys/dev/xen/console/xen_console.c and /usr/src/sys/kern/subr_prf.c
blindly use it to allocate a buffer on the kernel stack.

/usr/src/sys/geom/geom_subr.c and /usr/src/sys/geom/geom_io.c check whether
it's defined and set it to 64 if it isn't.  Otherwise it's simply used to
allocate a buffer on the kernel stack.

A user who doesn't really understand the purpose of PRINTF_BUFR_SIZE might
think "the bigger the better" and set it to be multi-megabytes in size.

I may be paranoid, but it seems like PRINTF_BUFR_SIZE should be checked
everywhere the way that cam_xpt.c does it.

--
Gary Jennejohn
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: PRINTF_BUFR_SIZE dangerous?

Gary Jennejohn-6
On Thu, 20 Aug 2020 08:33:32 +0200
Gary Jennejohn <[hidden email]> wrote:

> It seems like PRINTF_BUFR_SIZE is a kernel fault waiting to happen.
>
> Only /usr/src/sys/cam/cam_xpt.c asserts that it's <= a maximum value of
> 512 bytes.
>
> /usr/src/sys/kern/tty.c uses it to malloc space without checking its size.
>
> /usr/src/sys/dev/xen/console/xen_console.c and /usr/src/sys/kern/subr_prf.c
> blindly use it to allocate a buffer on the kernel stack.
>
> /usr/src/sys/geom/geom_subr.c and /usr/src/sys/geom/geom_io.c check whether
> it's defined and set it to 64 if it isn't.  Otherwise it's simply used to
> allocate a buffer on the kernel stack.
>
> A user who doesn't really understand the purpose of PRINTF_BUFR_SIZE might
> think "the bigger the better" and set it to be multi-megabytes in size.
>
> I may be paranoid, but it seems like PRINTF_BUFR_SIZE should be checked
> everywhere the way that cam_xpt.c does it.
>

OK, I decided to try setting PRINTF_BUFR_SIZE to (1024*1024) and the static
assert in /usr/src/sys/cam/cam_xpt.c saved the day.

Still, if a user isn't using scbus the problem would still exist.

--
Gary Jennejohn
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[hidden email]"