The arm64 architecture 64k page size enabled build failed on stable rc 5.5 CONFIG_ARM64_64K_PAGES=y CROSS_COMPILE=aarch64-linux-gnu- Toolchain gcc-9 In file included from ../block/scsi_ioctl.c:23: ../include/scsi/sg.h:75:2: error: unknown type name ‘compat_int_t’ compat_int_t interface_id; /* [i] 'S' for SCSI generic (required) */ ^~~~~~~~~~~~ ../include/scsi/sg.h:76:2: error: unknown type name ‘compat_int_t’ compat_int_t dxfer_direction; /* [i] data transfer direction */ ^~~~~~~~~~~~ ... ../include/scsi/sg.h:97:2: error: unknown type name ‘compat_uint_t’ compat_uint_t info; /* [o] auxiliary information */ ^~~~~~~~~~~~~ make[2]: *** [../scripts/Makefile.build:266: block/bsg.o] Error Ref: https://gitlab.com/Linaro/lkft/kernel-runs/-/jobs/431659186 On Mon, 10 Feb 2020 at 18:18, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > From: Arnd Bergmann <arnd@xxxxxxxx> > > commit 78ed001d9e7106171e0ee761cd854137dd731302 upstream. > > In the v5.4 merge window, a cleanup patch from Al Viro conflicted > with my rework of the compat handling for sg.c read(). Linus Torvalds > did a correct merge but pointed out that the resulting code is still > unsatisfactory. > > I later noticed that the sg_new_read() function still gets the compat > mode wrong, when the 'count' argument is large enough to pass a > compat_sg_io_hdr object, but not a nativ sg_io_hdr. > > To address both of these, move the definition of compat_sg_io_hdr > into a scsi/sg.h to make it visible to sg.c and rewrite the logic > for reading req_pack_id as well as the size check to a simpler > version that gets the expected results. > > Fixes: c35a5cfb4150 ("scsi: sg: sg_read(): simplify reading ->pack_id of userland sg_io_hdr_t") > Fixes: 98aaaec4a150 ("compat_ioctl: reimplement SG_IO handling") > Reviewed-by: Ben Hutchings <ben.hutchings@xxxxxxxxxxxxxxx> > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > --- > block/scsi_ioctl.c | 29 ------------ > drivers/scsi/sg.c | 126 ++++++++++++++++++++++++----------------------------- > include/scsi/sg.h | 30 ++++++++++++ > 3 files changed, 90 insertions(+), 95 deletions(-) > > --- a/block/scsi_ioctl.c > +++ b/block/scsi_ioctl.c > @@ -20,6 +20,7 @@ > #include <scsi/scsi.h> > #include <scsi/scsi_ioctl.h> > #include <scsi/scsi_cmnd.h> > +#include <scsi/sg.h> > > struct blk_cmd_filter { > unsigned long read_ok[BLK_SCSI_CMD_PER_LONG]; > @@ -550,34 +551,6 @@ static inline int blk_send_start_stop(st > return __blk_send_generic(q, bd_disk, GPCMD_START_STOP_UNIT, data); > } > > -#ifdef CONFIG_COMPAT > -struct compat_sg_io_hdr { > - compat_int_t interface_id; /* [i] 'S' for SCSI generic (required) */ > - compat_int_t dxfer_direction; /* [i] data transfer direction */ > - unsigned char cmd_len; /* [i] SCSI command length ( <= 16 bytes) */ > - unsigned char mx_sb_len; /* [i] max length to write to sbp */ > - unsigned short iovec_count; /* [i] 0 implies no scatter gather */ > - compat_uint_t dxfer_len; /* [i] byte count of data transfer */ > - compat_uint_t dxferp; /* [i], [*io] points to data transfer memory > - or scatter gather list */ > - compat_uptr_t cmdp; /* [i], [*i] points to command to perform */ > - compat_uptr_t sbp; /* [i], [*o] points to sense_buffer memory */ > - compat_uint_t timeout; /* [i] MAX_UINT->no timeout (unit: millisec) */ > - compat_uint_t flags; /* [i] 0 -> default, see SG_FLAG... */ > - compat_int_t pack_id; /* [i->o] unused internally (normally) */ > - compat_uptr_t usr_ptr; /* [i->o] unused internally */ > - unsigned char status; /* [o] scsi status */ > - unsigned char masked_status; /* [o] shifted, masked scsi status */ > - unsigned char msg_status; /* [o] messaging level data (optional) */ > - unsigned char sb_len_wr; /* [o] byte count actually written to sbp */ > - unsigned short host_status; /* [o] errors from host adapter */ > - unsigned short driver_status; /* [o] errors from software driver */ > - compat_int_t resid; /* [o] dxfer_len - actual_transferred */ > - compat_uint_t duration; /* [o] time taken by cmd (unit: millisec) */ > - compat_uint_t info; /* [o] auxiliary information */ > -}; > -#endif > - > int put_sg_io_hdr(const struct sg_io_hdr *hdr, void __user *argp) > { > #ifdef CONFIG_COMPAT > --- a/drivers/scsi/sg.c > +++ b/drivers/scsi/sg.c > @@ -405,6 +405,38 @@ sg_release(struct inode *inode, struct f > return 0; > } > > +static int get_sg_io_pack_id(int *pack_id, void __user *buf, size_t count) > +{ > + struct sg_header __user *old_hdr = buf; > + int reply_len; > + > + if (count >= SZ_SG_HEADER) { > + /* negative reply_len means v3 format, otherwise v1/v2 */ > + if (get_user(reply_len, &old_hdr->reply_len)) > + return -EFAULT; > + > + if (reply_len >= 0) > + return get_user(*pack_id, &old_hdr->pack_id); > + > + if (in_compat_syscall() && > + count >= sizeof(struct compat_sg_io_hdr)) { > + struct compat_sg_io_hdr __user *hp = buf; > + > + return get_user(*pack_id, &hp->pack_id); > + } > + > + if (count >= sizeof(struct sg_io_hdr)) { > + struct sg_io_hdr __user *hp = buf; > + > + return get_user(*pack_id, &hp->pack_id); > + } > + } > + > + /* no valid header was passed, so ignore the pack_id */ > + *pack_id = -1; > + return 0; > +} > + > static ssize_t > sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos) > { > @@ -413,8 +445,8 @@ sg_read(struct file *filp, char __user * > Sg_request *srp; > int req_pack_id = -1; > sg_io_hdr_t *hp; > - struct sg_header *old_hdr = NULL; > - int retval = 0; > + struct sg_header *old_hdr; > + int retval; > > /* > * This could cause a response to be stranded. Close the associated > @@ -429,79 +461,34 @@ sg_read(struct file *filp, char __user * > SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp, > "sg_read: count=%d\n", (int) count)); > > - if (sfp->force_packid && (count >= SZ_SG_HEADER)) { > - old_hdr = memdup_user(buf, SZ_SG_HEADER); > - if (IS_ERR(old_hdr)) > - return PTR_ERR(old_hdr); > - if (old_hdr->reply_len < 0) { > - if (count >= SZ_SG_IO_HDR) { > - /* > - * This is stupid. > - * > - * We're copying the whole sg_io_hdr_t from user > - * space just to get the 'pack_id' field. But the > - * field is at different offsets for the compat > - * case, so we'll use "get_sg_io_hdr()" to copy > - * the whole thing and convert it. > - * > - * We could do something like just calculating the > - * offset based of 'in_compat_syscall()', but the > - * 'compat_sg_io_hdr' definition is in the wrong > - * place for that. > - */ > - sg_io_hdr_t *new_hdr; > - new_hdr = kmalloc(SZ_SG_IO_HDR, GFP_KERNEL); > - if (!new_hdr) { > - retval = -ENOMEM; > - goto free_old_hdr; > - } > - retval = get_sg_io_hdr(new_hdr, buf); > - req_pack_id = new_hdr->pack_id; > - kfree(new_hdr); > - if (retval) { > - retval = -EFAULT; > - goto free_old_hdr; > - } > - } > - } else > - req_pack_id = old_hdr->pack_id; > - } > + if (sfp->force_packid) > + retval = get_sg_io_pack_id(&req_pack_id, buf, count); > + if (retval) > + return retval; > + > srp = sg_get_rq_mark(sfp, req_pack_id); > if (!srp) { /* now wait on packet to arrive */ > - if (atomic_read(&sdp->detaching)) { > - retval = -ENODEV; > - goto free_old_hdr; > - } > - if (filp->f_flags & O_NONBLOCK) { > - retval = -EAGAIN; > - goto free_old_hdr; > - } > + if (atomic_read(&sdp->detaching)) > + return -ENODEV; > + if (filp->f_flags & O_NONBLOCK) > + return -EAGAIN; > retval = wait_event_interruptible(sfp->read_wait, > (atomic_read(&sdp->detaching) || > (srp = sg_get_rq_mark(sfp, req_pack_id)))); > - if (atomic_read(&sdp->detaching)) { > - retval = -ENODEV; > - goto free_old_hdr; > - } > - if (retval) { > + if (atomic_read(&sdp->detaching)) > + return -ENODEV; > + if (retval) > /* -ERESTARTSYS as signal hit process */ > - goto free_old_hdr; > - } > - } > - if (srp->header.interface_id != '\0') { > - retval = sg_new_read(sfp, buf, count, srp); > - goto free_old_hdr; > + return retval; > } > + if (srp->header.interface_id != '\0') > + return sg_new_read(sfp, buf, count, srp); > > hp = &srp->header; > - if (old_hdr == NULL) { > - old_hdr = kmalloc(SZ_SG_HEADER, GFP_KERNEL); > - if (! old_hdr) { > - retval = -ENOMEM; > - goto free_old_hdr; > - } > - } > - memset(old_hdr, 0, SZ_SG_HEADER); > + old_hdr = kzalloc(SZ_SG_HEADER, GFP_KERNEL); > + if (!old_hdr) > + return -ENOMEM; > + > old_hdr->reply_len = (int) hp->timeout; > old_hdr->pack_len = old_hdr->reply_len; /* old, strange behaviour */ > old_hdr->pack_id = hp->pack_id; > @@ -575,7 +562,12 @@ sg_new_read(Sg_fd * sfp, char __user *bu > int err = 0, err2; > int len; > > - if (count < SZ_SG_IO_HDR) { > + if (in_compat_syscall()) { > + if (count < sizeof(struct compat_sg_io_hdr)) { > + err = -EINVAL; > + goto err_out; > + } > + } else if (count < SZ_SG_IO_HDR) { > err = -EINVAL; > goto err_out; > } > --- a/include/scsi/sg.h > +++ b/include/scsi/sg.h > @@ -68,6 +68,36 @@ typedef struct sg_io_hdr > unsigned int info; /* [o] auxiliary information */ > } sg_io_hdr_t; /* 64 bytes long (on i386) */ > > +#if defined(__KERNEL__) > +#include <linux/compat.h> > + > +struct compat_sg_io_hdr { > + compat_int_t interface_id; /* [i] 'S' for SCSI generic (required) */ > + compat_int_t dxfer_direction; /* [i] data transfer direction */ > + unsigned char cmd_len; /* [i] SCSI command length ( <= 16 bytes) */ > + unsigned char mx_sb_len; /* [i] max length to write to sbp */ > + unsigned short iovec_count; /* [i] 0 implies no scatter gather */ > + compat_uint_t dxfer_len; /* [i] byte count of data transfer */ > + compat_uint_t dxferp; /* [i], [*io] points to data transfer memory > + or scatter gather list */ > + compat_uptr_t cmdp; /* [i], [*i] points to command to perform */ > + compat_uptr_t sbp; /* [i], [*o] points to sense_buffer memory */ > + compat_uint_t timeout; /* [i] MAX_UINT->no timeout (unit: millisec) */ > + compat_uint_t flags; /* [i] 0 -> default, see SG_FLAG... */ > + compat_int_t pack_id; /* [i->o] unused internally (normally) */ > + compat_uptr_t usr_ptr; /* [i->o] unused internally */ > + unsigned char status; /* [o] scsi status */ > + unsigned char masked_status; /* [o] shifted, masked scsi status */ > + unsigned char msg_status; /* [o] messaging level data (optional) */ > + unsigned char sb_len_wr; /* [o] byte count actually written to sbp */ > + unsigned short host_status; /* [o] errors from host adapter */ > + unsigned short driver_status; /* [o] errors from software driver */ > + compat_int_t resid; /* [o] dxfer_len - actual_transferred */ > + compat_uint_t duration; /* [o] time taken by cmd (unit: millisec) */ > + compat_uint_t info; /* [o] auxiliary information */ > +}; > +#endif > + > #define SG_INTERFACE_ID_ORIG 'S' > > /* Use negative values to flag difference from original sg_header structure */ > > -- Linaro LKFT https://lkft.linaro.org
