Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 18 Dec 2021 14:08:00 -0500
From:      Shawn Webb <shawn.webb@hardenedbsd.org>
To:        Brooks Davis <brooks@FreeBSD.org>
Cc:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   Re: git: 45b48cbc2b58 - main - usb: real freebsd32 support for most ioctls
Message-ID:  <20211218190800.2v7bzacypzhiguvz@mutt-hbsd>
In-Reply-To: <202112172128.1BHLSvgR063035@gitrepo.freebsd.org>
References:  <202112172128.1BHLSvgR063035@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--mej7q4nqvusyqfqy
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hey Brooks,

On Fri, Dec 17, 2021 at 09:28:57PM +0000, Brooks Davis wrote:
> The branch main has been updated by brooks:
>=20
> URL: https://cgit.FreeBSD.org/src/commit/?id=3D45b48cbc2b5819cd6e3dee3632=
d66e55d5d7c101
>=20
> 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
>=20
>     usb: real freebsd32 support for most ioctls
>    =20
>     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.
>    =20
>     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.
>    =20
>     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(-)
>=20
> 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"
> =20
>  #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;
>  };
> =20
> +#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);
>  }
> =20
> +#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 =3D 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 a=
ddr, int flag,
>  	if (sc =3D=3D NULL)
>  		return (EIO);
> =20
> +#ifdef COMPAT_FREEBSD32
> +	hgd =3D (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 =3D _IOC_NEWTYPE(cmd, struct hidraw_gen_descriptor);
> +		hgd32 =3D (struct hidraw_gen_descriptor32 *)addr;
> +		hgd =3D &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=3D/usr/obj/usr/src/arm64.a=
arch64/tmp -B/usr/obj/usr/src/arm64.aarch64/tmp/usr/bin  -O2 -pipe -fno-com=
mon -DHARDE
NEDBSD  -fno-strict-aliasing -Werror -D_KERNEL -DKLD_MODULE -DKLD_TIED -nos=
tdinc   -DHAVE_KERNEL_OPTION_HEADERS -include /usr/obj/usr/src/arm64.aarch6=
4/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=3D./machine=3D/usr/src/sys/arm64/include -I/usr/obj/usr/src/arm64.=
aarch64/sys/HARDENEDBSD   -mstack-protector-guard=3Dsysreg -mstack-protecto=
r-guard-reg=3Dsp_
el0 -mstack-protector-guard-offset=3D0  -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 -Wpo=
inter-arith -Wcast-qual -Wundef -Wno-pointer-sign -D__printf__=3D__freebsd_=
kprintf__ -
Wmissing-include-dirs -fdiagnostics-show-option -Wno-unknown-pragmas -Wno-e=
rror=3Dtautological-compare -Wno-error=3Dempty-body -Wno-error=3Dparenthese=
s-equality -Wno
-error=3Dunused-function -Wno-error=3Dpointer-sign -Wno-error=3Dshift-negat=
ive-value -Wno-address-of-packed-member -Wno-error=3Dunused-but-set-variabl=
e -Wno-format-zer
o-length     -std=3Diso9899:1999 -c /usr/src/sys/dev/hid/hidraw.c -o hidraw=
=2Eo
/usr/src/sys/dev/hid/hidraw.c:643:27: error: variable 'hgd' is uninitialize=
d 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' t=
o silence this warning
        struct hidraw_gen_descriptor *hgd;


Thanks,

--=20
Shawn Webb
Cofounder / Security Engineer
HardenedBSD

https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/raw/master/Shawn_Webb/03A=
4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc

--mej7q4nqvusyqfqy
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEA6TL67gupaZ9nzhT/y5nonf44foFAmG+MYoACgkQ/y5nonf4
4fom5A/+PMnbuLxFngYIX5Vo0pzGDUM1Zw+Hx2XoSjOZyFSTi2fwiEqHRQH2yuEi
AmqkG/+jt6mNEMX987Djs0jQO+yWgUWi50jRXdbECHdt9/yUQFv61CPSSdBJWcZs
UWoyDMH8n0C6t7FPRhnpQgkleP5Wg6Z/KIe/fH/rkiLWn7U4Kqp3GTbhTf55G3Zn
Vqdh/XS5vSjP1GxnE0knQZ5kP5xbJ83B0ZtizwqfTgfQRBsL7j7nLpFZXdiH1H3f
22heTZ+oxACSaL6uWjU5b8QaHuDetIUbWPVL1ZuwZzvELFwnCJImAOezPJwCjjR4
Qvopivb7mGKw6AvBc5PBKInaWGfqA9W7/fxq4dy/n5u1rrGzyYeJFG8hlVFBRFo6
mZv6dfJVdmrl/qSIhTCmONmV4yGDmNSM+/VBbpjcjwv3IWtd4/FNTCWLTMRlx+u6
xbPoBnwafJaGkId43QjTV2bz12qjwG0b1XUmBLPUDrk/wxpo0D3w8G8Va1lYKMqi
6ajhI0vgZMgVo82IdkzpkhW+q4apm9MthtHgdFGRVymFKhpwuc+S1QT9EXmocS8k
7rGFceeausKNKIo5pwf6Fnri3pitdmpJM0BC9hcEbJU6qzrPuCkFgusPU9A8IRzf
yp2/LxXTuDMBW5cnwmhYFGORety/X2d4bVxtfgpN7aCIWoxbVfo=
=wU97
-----END PGP SIGNATURE-----

--mej7q4nqvusyqfqy--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20211218190800.2v7bzacypzhiguvz>