Date: Wed, 30 May 2012 16:13:35 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r236319 - stable/8/sys/dev/amr Message-ID: <201205301613.q4UGDZcY061632@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jhb Date: Wed May 30 16:13:34 2012 New Revision: 236319 URL: http://svn.freebsd.org/changeset/base/236319 Log: MFC 234501: The amr(4) firmware contains a rather dubious "feature" where it assumes for small buffers (< 64k) that the OS driver is actually using a buffer rounded up to the next power of 2. It also assumes that the buffer is at least 4k in size. Furthermore, there is at least one known instance of megarc sending a request with a 12k buffer where the firmware writes out a 24k-ish reply. To workaround the data corruption triggered by this "feature", ensure that buffers for user commands use a minimum size of 32k, and that buffers between 32k and 64k use a 64k buffer. Modified: stable/8/sys/dev/amr/amr.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/boot/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/e1000/ (props changed) Modified: stable/8/sys/dev/amr/amr.c ============================================================================== --- stable/8/sys/dev/amr/amr.c Wed May 30 16:13:10 2012 (r236318) +++ stable/8/sys/dev/amr/amr.c Wed May 30 16:13:34 2012 (r236319) @@ -535,6 +535,31 @@ shutdown_out: amr_startup(sc); } +/* + * Bug-for-bug compatibility with Linux! + * Some apps will send commands with inlen and outlen set to 0, + * even though they expect data to be transfered to them from the + * card. Linux accidentally allows this by allocating a 4KB + * buffer for the transfer anyways, but it then throws it away + * without copying it back to the app. + * + * The amr(4) firmware relies on this feature. In fact, it assumes + * the buffer is always a power of 2 up to a max of 64k. There is + * also at least one case where it assumes a buffer less than 16k is + * greater than 16k. Force a minimum buffer size of 32k and round + * sizes between 32k and 64k up to 64k as a workaround. + */ +static unsigned long +amr_ioctl_buffer_length(unsigned long len) +{ + + if (len <= 32 * 1024) + return (32 * 1024); + if (len <= 64 * 1024) + return (64 * 1024); + return (len); +} + int amr_linux_ioctl_int(struct cdev *dev, u_long cmd, caddr_t addr, int32_t flag, struct thread *td) @@ -664,16 +689,7 @@ amr_linux_ioctl_int(struct cdev *dev, u_ error = ENOIOCTL; break; } else { - /* - * Bug-for-bug compatibility with Linux! - * Some apps will send commands with inlen and outlen set to 0, - * even though they expect data to be transfered to them from the - * card. Linux accidentally allows this by allocating a 4KB - * buffer for the transfer anyways, but it then throws it away - * without copying it back to the app. - */ - if (!len) - len = 4096; + len = amr_ioctl_buffer_length(imax(ali.inlen, ali.outlen)); dp = malloc(len, M_AMR, M_WAITOK | M_ZERO); @@ -703,7 +719,7 @@ amr_linux_ioctl_int(struct cdev *dev, u_ status = ac->ac_status; error = copyout(&status, &((struct amr_mailbox *)&((struct amr_linux_ioctl *)addr)->mbox[0])->mb_status, sizeof(status)); if (ali.outlen) { - error = copyout(dp, (void *)(uintptr_t)mb->mb_physaddr, len); + error = copyout(dp, (void *)(uintptr_t)mb->mb_physaddr, ali.outlen); if (error) break; } @@ -750,7 +766,7 @@ amr_ioctl(struct cdev *dev, u_long cmd, struct amr_command *ac; struct amr_mailbox_ioctl *mbi; void *dp, *au_buffer; - unsigned long au_length; + unsigned long au_length, real_length; unsigned char *au_cmd; int *au_statusp, au_direction; int error; @@ -842,8 +858,9 @@ amr_ioctl(struct cdev *dev, u_long cmd, } /* handle inbound data buffer */ + real_length = amr_ioctl_buffer_length(au_length); if (au_length != 0 && au_cmd[0] != 0x06) { - if ((dp = malloc(au_length, M_AMR, M_WAITOK|M_ZERO)) == NULL) { + if ((dp = malloc(real_length, M_AMR, M_WAITOK|M_ZERO)) == NULL) { error = ENOMEM; goto out; } @@ -902,7 +919,7 @@ amr_ioctl(struct cdev *dev, u_long cmd, /* build the command */ ac->ac_data = dp; - ac->ac_length = au_length; + ac->ac_length = real_length; ac->ac_flags |= AMR_CMD_DATAIN|AMR_CMD_DATAOUT; /* run the command */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201205301613.q4UGDZcY061632>