Re: git: 45b48cbc2b58 - main - usb: real freebsd32 support for most ioctls
Date: Sat, 18 Dec 2021 19:08:00 UTC
Hey Brooks,
On Fri, Dec 17, 2021 at 09:28:57PM +0000, Brooks Davis wrote:
> The branch main has been updated by brooks:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=45b48cbc2b5819cd6e3dee3632d66e55d5d7c101
>
> commit 45b48cbc2b5819cd6e3dee3632d66e55d5d7c101
> Author: Brooks Davis <brooks@FreeBSD.org>
> AuthorDate: 2021-12-17 21:28:13 +0000
> Commit: Brooks Davis <brooks@FreeBSD.org>
> CommitDate: 2021-12-17 21:28:13 +0000
>
> usb: real freebsd32 support for most ioctls
>
> Use thunks or alternative access methods to support ioctls without
> the COMPAT_32BIT hacks that store pointers in uint64_t's on 32-bit
> platforms. This should allow a normal i386 libusb to work.
>
> On CheriBSD, the sizes of the structs will differ between CheriABI
> (the default) and freebsd64 no matter what so we need proper compat
> support there. This change paves the way.
>
> Reviewed by: hselasky, jrtc27 (prior version)
> ---
> sys/dev/hid/hidraw.c | 93 +++++++++++++++++++++++++++++++--
> sys/dev/usb/input/uhid.c | 25 +++++++--
> sys/dev/usb/input/uhid_snes.c | 26 ++++++++--
> sys/dev/usb/usb_dev.c | 12 +++++
> sys/dev/usb/usb_generic.c | 118 ++++++++++++++++++++++++++++++++++++++++++
> sys/dev/usb/usb_ioctl.h | 57 ++++++++++++++++++++
> 6 files changed, 321 insertions(+), 10 deletions(-)
>
> diff --git a/sys/dev/hid/hidraw.c b/sys/dev/hid/hidraw.c
> index e71b2e2c7d5d..f359fe3e7e6f 100644
> --- a/sys/dev/hid/hidraw.c
> +++ b/sys/dev/hid/hidraw.c
> @@ -41,6 +41,9 @@ __FBSDID("$FreeBSD$");
> #include "opt_hid.h"
>
> #include <sys/param.h>
> +#ifdef COMPAT_FREEBSD32
> +#include <sys/abi_compat.h>
> +#endif
> #include <sys/bus.h>
> #include <sys/conf.h>
> #include <sys/fcntl.h>
> @@ -115,6 +118,31 @@ struct hidraw_softc {
> struct cdev *dev;
> };
>
> +#ifdef COMPAT_FREEBSD32
> +struct hidraw_gen_descriptor32 {
> + uint32_t hgd_data; /* void * */
> + uint16_t hgd_lang_id;
> + uint16_t hgd_maxlen;
> + uint16_t hgd_actlen;
> + uint16_t hgd_offset;
> + uint8_t hgd_config_index;
> + uint8_t hgd_string_index;
> + uint8_t hgd_iface_index;
> + uint8_t hgd_altif_index;
> + uint8_t hgd_endpt_index;
> + uint8_t hgd_report_type;
> + uint8_t reserved[8];
> +};
> +#define HIDRAW_GET_REPORT_DESC32 \
> + _IOC_NEWTYPE(HIDRAW_GET_REPORT_DESC, struct hidraw_gen_descriptor32)
> +#define HIDRAW_GET_REPORT32 \
> + _IOC_NEWTYPE(HIDRAW_GET_REPORT, struct hidraw_gen_descriptor32)
> +#define HIDRAW_SET_REPORT_DESC32 \
> + _IOC_NEWTYPE(HIDRAW_SET_REPORT_DESC, struct hidraw_gen_descriptor32)
> +#define HIDRAW_SET_REPORT32 \
> + _IOC_NEWTYPE(HIDRAW_SET_REPORT, struct hidraw_gen_descriptor32)
> +#endif
> +
> static d_open_t hidraw_open;
> static d_read_t hidraw_read;
> static d_write_t hidraw_write;
> @@ -507,11 +535,35 @@ hidraw_write(struct cdev *dev, struct uio *uio, int flag)
> return (error);
> }
>
> +#ifdef COMPAT_FREEBSD32
> +static void
> +update_hgd32(const struct hidraw_gen_descriptor *hgd,
> + struct hidraw_gen_descriptor32 *hgd32)
> +{
> + /* Don't update hgd_data pointer */
> + CP(*hgd, *hgd32, hgd_lang_id);
> + CP(*hgd, *hgd32, hgd_maxlen);
> + CP(*hgd, *hgd32, hgd_actlen);
> + CP(*hgd, *hgd32, hgd_offset);
> + CP(*hgd, *hgd32, hgd_config_index);
> + CP(*hgd, *hgd32, hgd_string_index);
> + CP(*hgd, *hgd32, hgd_iface_index);
> + CP(*hgd, *hgd32, hgd_altif_index);
> + CP(*hgd, *hgd32, hgd_endpt_index);
> + CP(*hgd, *hgd32, hgd_report_type);
> + /* Don't update reserved */
> +}
> +#endif
> +
> static int
> hidraw_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
> struct thread *td)
> {
> uint8_t local_buf[HIDRAW_LOCAL_BUFSIZE];
> +#ifdef COMPAT_FREEBSD32
> + struct hidraw_gen_descriptor local_hgd;
> + struct hidraw_gen_descriptor32 *hgd32 = NULL;
> +#endif
> void *buf;
> struct hidraw_softc *sc;
> struct hidraw_gen_descriptor *hgd;
> @@ -527,6 +579,32 @@ hidraw_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
> if (sc == NULL)
> return (EIO);
>
> +#ifdef COMPAT_FREEBSD32
> + hgd = (struct hidraw_gen_descriptor *)addr;
> + switch (cmd) {
> + case HIDRAW_GET_REPORT_DESC32:
> + case HIDRAW_GET_REPORT32:
> + case HIDRAW_SET_REPORT_DESC32:
> + case HIDRAW_SET_REPORT32:
> + cmd = _IOC_NEWTYPE(cmd, struct hidraw_gen_descriptor);
> + hgd32 = (struct hidraw_gen_descriptor32 *)addr;
> + hgd = &local_hgd;
> + PTRIN_CP(*hgd32, *hgd, hgd_data);
> + CP(*hgd32, *hgd, hgd_lang_id);
> + CP(*hgd32, *hgd, hgd_maxlen);
> + CP(*hgd32, *hgd, hgd_actlen);
> + CP(*hgd32, *hgd, hgd_offset);
> + CP(*hgd32, *hgd, hgd_config_index);
> + CP(*hgd32, *hgd, hgd_string_index);
> + CP(*hgd32, *hgd, hgd_iface_index);
> + CP(*hgd32, *hgd, hgd_altif_index);
> + CP(*hgd32, *hgd, hgd_endpt_index);
> + CP(*hgd32, *hgd, hgd_report_type);
> + /* Don't copy reserved */
> + break;
> + }
> +#endif
> +
It appears this broke building on arm64 when COMPAT_FREEBSD32 is
disabled:
cc -target aarch64-unknown-freebsd14.0 --sysroot=/usr/obj/usr/src/arm64.aarch64/tmp -B/usr/obj/usr/src/arm64.aarch64/tmp/usr/bin -O2 -pipe -fno-common -DHARDE
NEDBSD -fno-strict-aliasing -Werror -D_KERNEL -DKLD_MODULE -DKLD_TIED -nostdinc -DHAVE_KERNEL_OPTION_HEADERS -include /usr/obj/usr/src/arm64.aarch64/sys/HAR
DENEDBSD/opt_global.h -I. -I/usr/src/sys -I/usr/src/sys/contrib/ck/include -fno-common -g -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fPIC -fdebug-pr
efix-map=./machine=/usr/src/sys/arm64/include -I/usr/obj/usr/src/arm64.aarch64/sys/HARDENEDBSD -mstack-protector-guard=sysreg -mstack-protector-guard-reg=sp_
el0 -mstack-protector-guard-offset=0 -MD -MF.depend.hidraw.o -MThidraw.o -mgeneral-regs-only -ffixed-x18 -ffreestanding -fwrapv -fstack-protector -Wall -Wred
undant-decls -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wcast-qual -Wundef -Wno-pointer-sign -D__printf__=__freebsd_kprintf__ -
Wmissing-include-dirs -fdiagnostics-show-option -Wno-unknown-pragmas -Wno-error=tautological-compare -Wno-error=empty-body -Wno-error=parentheses-equality -Wno
-error=unused-function -Wno-error=pointer-sign -Wno-error=shift-negative-value -Wno-address-of-packed-member -Wno-error=unused-but-set-variable -Wno-format-zer
o-length -std=iso9899:1999 -c /usr/src/sys/dev/hid/hidraw.c -o hidraw.o
/usr/src/sys/dev/hid/hidraw.c:643:27: error: variable 'hgd' is uninitialized when used here [-Werror,-Wuninitialized]
if (sc->sc_rdesc->len > hgd->hgd_maxlen) {
^~~
/usr/src/sys/dev/hid/hidraw.c:569:35: note: initialize the variable 'hgd' to silence this warning
struct hidraw_gen_descriptor *hgd;
Thanks,
--
Shawn Webb
Cofounder / Security Engineer
HardenedBSD
https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/raw/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc