[PATCH] scsi: avoid a double-fetch and a redundant copy
What we need is only "pack_id", so do not create a heap object or copy
the whole object in. The fix efficiently copies "pack_id" only.
Signed-off-by: Kangjie Lu
---
drivers/scsi/sg.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index c6ad00703c5b..4dacbfffd113 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -446,16 +446,8 @@ sg_read(struct file *filp, char __user *buf, size_t count,
loff_t * ppos)
}
if (old_hdr->reply_len < 0) {
if (count >= SZ_SG_IO_HDR) {
- 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 =__copy_from_user
- (new_hdr, buf, SZ_SG_IO_HDR);
- req_pack_id = new_hdr->pack_id;
- kfree(new_hdr);
+ retval = get_user(req_pack_id,
+ &((sg_io_hdr_t *)buf->pack_id));
if (retval) {
retval = -EFAULT;
goto free_old_hdr;
--
2.17.2 (Apple Git-113)
[PATCH] scsi: fix a double-fetch bug in sg_write
"opcode" has been copied in from user space and checked. We should not copy it in again, which may have been modified by malicous multi-threading user programs through race conditions. The fix uses the opcode fetched in the first copy. Signed-off-by: Kangjie Lu --- drivers/scsi/sg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 4dacbfffd113..41774e4f9508 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -686,7 +686,8 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos) hp->flags = input_size; /* structure abuse ... */ hp->pack_id = old_hdr.pack_id; hp->usr_ptr = NULL; - if (__copy_from_user(cmnd, buf, cmd_size)) + cmnd[0] = opcode; + if (__copy_from_user(cmnd + 1, buf + 1, cmd_size - 1)) return -EFAULT; /* * SG_DXFER_TO_FROM_DEV is functionally equivalent to SG_DXFER_FROM_DEV, -- 2.17.2 (Apple Git-113)
[PATCH] scsi: avoiding fetching signature from user space again after check
The signature is checked so that it must be "MEGANIT". After the check,
if we fetch the signature again from user space, it may have been
modified by malicious user programs through race conditions. The fix
avoids fetching the signature again.
Signed-off-by: Kangjie Lu
---
drivers/scsi/megaraid.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 8c7154143a4e..a2255fbd0ab6 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -3396,7 +3396,6 @@ static int
mega_m_to_n(void __user *arg, nitioctl_t *uioc)
{
struct uioctl_t uioc_mimd;
- charsignature[8] = {0};
u8 opcode;
u8 subopcode;
@@ -3408,10 +3407,10 @@ mega_m_to_n(void __user *arg, nitioctl_t *uioc)
* beginning of the structure.
*/
- if( copy_from_user(signature, arg, 7) )
+ if (copy_from_user(&uioc_mimd, arg, 7))
return (-EFAULT);
- if( memcmp(signature, "MEGANIT", 7) == 0 ) {
+ if (memcmp(&uioc_mimd, "MEGANIT", 7) == 0) {
/*
* NOTE NOTE: The nit ioctl is still under flux because of
@@ -3421,7 +3420,7 @@ mega_m_to_n(void __user *arg, nitioctl_t *uioc)
*/
return -EINVAL;
#if 0
- if( copy_from_user(uioc, arg, sizeof(nitioctl_t)) )
+ if (copy_from_user(uioc, arg, sizeof(nitioctl_t)))
return (-EFAULT);
return 0;
#endif
@@ -3432,7 +3431,10 @@ mega_m_to_n(void __user *arg, nitioctl_t *uioc)
*
* Get the user ioctl structure
*/
- if( copy_from_user(&uioc_mimd, arg, sizeof(struct uioctl_t)) )
+ if (copy_from_user((char *)&uioc_mimd + sizeof(uioc->signature),
+ arg + sizeof(uioc->signature),
+ sizeof(struct uioctl_t) -
+ sizeof(uioc->signature)))
return (-EFAULT);
--
2.17.2 (Apple Git-113)
[PATCH] scsi: aacraid: fix a potential data inconsistency caused by double-fetch
"user_srb->count" may be changed by malicious user races. Let's set
"user_srbcmd->count" fetched in the second copy to be the one fetched in
the first copy.
Signed-off-by: Kangjie Lu
---
drivers/scsi/aacraid/commctrl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 25f6600d6c09..eb18117c431a 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -539,6 +539,8 @@ static int aac_send_raw_srb(struct aac_dev* dev, void
__user * arg)
rcode = -EFAULT;
goto cleanup;
}
+ /* Ensure user_srb->count is not changed */
+ user_srbcmd->count = fibsize;
flags = user_srbcmd->flags; /* from user in cpu order */
switch (flags & (SRB_DataIn | SRB_DataOut)) {
--
2.17.2 (Apple Git-113)
[PATCH] scsi: fix a double-fetch bug in adpt_i2o_passthru
user_msg[0] is a size variable, which is copied in from user space and
checked. It is copied in again from user space after the check, and used
in the following execution. Malicious user programs can race to change
user_msg[0] between the two copies, leading to incorrect size. The fix
ensures to use the checked size.
Signed-off-by: Kangjie Lu
---
drivers/scsi/dpt_i2o.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 37de8fb186d7..93bd1d1bd5b5 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -1733,6 +1733,9 @@ static int adpt_i2o_passthru(adpt_hba* pHba, u32 __user
*arg)
if(copy_from_user(msg, user_msg, size)) {
return -EFAULT;
}
+ /* Ensure it is not changed in the second copy */
+ msg[0] = size;
+
get_user(reply_size, &user_reply[0]);
reply_size = reply_size>>16;
if(reply_size > REPLY_FRAME_SIZE){
--
2.17.2 (Apple Git-113)
[PATCH] scsi: a potential double-fetch bug when copying msg
user_msg[0] is copied in twice from user space. It contains size and is critical. The fix ensures it is not changed in the second copy. Signed-off-by: Kangjie Lu --- drivers/scsi/dpt_i2o.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c index 93bd1d1bd5b5..2294520842e0 100644 --- a/drivers/scsi/dpt_i2o.c +++ b/drivers/scsi/dpt_i2o.c @@ -1850,6 +1850,8 @@ static int adpt_i2o_passthru(adpt_hba* pHba, u32 __user *arg) rcode = -EFAULT; goto cleanup; } + /* Ensure it is not changed in the second copy */ + msg[0] = size; sg_count = (size - sg_offset*4) / sizeof(struct sg_simple_element); // TODO add 64 bit API -- 2.17.2 (Apple Git-113)
Re: [PATCH] scsi: avoid a double-fetch and a redundant copy
Hi Kangjie,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on scsi/for-next]
[also build test ERROR on v4.20 next-20181224]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Kangjie-Lu/scsi-avoid-a-double-fetch-and-a-redundant-copy/20181226-042018
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: i386-randconfig-x079-201851 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from include/linux/uaccess.h:14:0,
from include/linux/poll.h:12,
from drivers//scsi/sg.c:42:
drivers//scsi/sg.c: In function 'sg_read':
>> drivers//scsi/sg.c:450:27: error: request for member 'pack_id' in something
>> not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/x86/include/asm/uaccess.h:138:41: note: in definition of macro
'__inttype'
__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
^
>> drivers//scsi/sg.c:449:14: note: in expansion of macro 'get_user'
retval = get_user(req_pack_id,
^~~~
>> arch/x86/include/asm/uaccess.h:138:12: error: first argument to
>> '__builtin_choose_expr' not a constant
__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
^
>> arch/x86/include/asm/uaccess.h:174:11: note: in expansion of macro
>> '__inttype'
register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX); \
^
>> drivers//scsi/sg.c:449:14: note: in expansion of macro 'get_user'
retval = get_user(req_pack_id,
^~~~
>> drivers//scsi/sg.c:450:27: error: request for member 'pack_id' in something
>> not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/x86/include/asm/uaccess.h:180:15: note: in definition of macro
'get_user'
: "0" (ptr), "i" (sizeof(*(ptr; \
^~~
>> drivers//scsi/sg.c:450:27: error: request for member 'pack_id' in something
>> not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/x86/include/asm/uaccess.h:180:35: note: in definition of macro
'get_user'
: "0" (ptr), "i" (sizeof(*(ptr; \
^~~
>> drivers//scsi/sg.c:450:27: error: request for member 'pack_id' in something
>> not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/x86/include/asm/uaccess.h:181:30: note: in definition of macro
'get_user'
(x) = (__force __typeof__(*(ptr))) __val_gu; \
^~~
--
In file included from include/linux/uaccess.h:14:0,
from include/linux/poll.h:12,
from drivers/scsi/sg.c:42:
drivers/scsi/sg.c: In function 'sg_read':
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/x86/include/asm/uaccess.h:138:41: note: in definition of macro
'__inttype'
__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
^
drivers/scsi/sg.c:449:14: note: in expansion of macro 'get_user'
retval = get_user(req_pack_id,
^~~~
>> arch/x86/include/asm/uaccess.h:138:12: error: first argument to
>> '__builtin_choose_expr' not a constant
__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
^
>> arch/x86/include/asm/uaccess.h:174:11: note: in expansion of macro
>> '__inttype'
register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX); \
^
drivers/scsi/sg.c:449:14: note: in expansion of macro 'get_user'
retval = get_user(req_pack_id,
^~~~
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/x86/include/asm/uaccess.h:180:15: note: in definition of macro
'get_user'
: "0" (ptr), "i" (sizeof(*(ptr; \
^~~
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/x86/include/asm/uaccess.h:180:35: note: in definition of macro
'get_user'
: "0" (ptr), "i" (sizeof(*(ptr; \
^~~
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
&((s
Re: [PATCH] scsi: avoid a double-fetch and a redundant copy
Hi Kangjie,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.20 next-20181224]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Kangjie-Lu/scsi-avoid-a-double-fetch-and-a-redundant-copy/20181226-042018
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=m68k
All warnings (new ones prefixed by >>):
In file included from arch/m68k/include/asm/uaccess.h:5:0,
from include/linux/uaccess.h:14,
from include/linux/poll.h:12,
from drivers/scsi/sg.c:42:
drivers/scsi/sg.c: In function 'sg_read':
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/m68k/include/asm/uaccess_mm.h:134:19: note: in definition of macro
'__get_user'
switch (sizeof(*(ptr))) { \
^~~
drivers/scsi/sg.c:449:14: note: in expansion of macro 'get_user'
retval = get_user(req_pack_id,
^~~~
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/m68k/include/asm/uaccess_mm.h:126:12: note: in definition of macro
'__get_user_asm'
: "m" (*(ptr)), "i" (err));\
^~~
>> arch/m68k/include/asm/uaccess_mm.h:180:26: note: in expansion of macro
>> '__get_user'
#define get_user(x, ptr) __get_user(x, ptr)
^~
drivers/scsi/sg.c:449:14: note: in expansion of macro 'get_user'
retval = get_user(req_pack_id,
^~~~
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/m68k/include/asm/uaccess_mm.h:127:26: note: in definition of macro
'__get_user_asm'
(x) = (__force typeof(*(ptr)))(__force unsigned long)__gu_val; \
^~~
>> arch/m68k/include/asm/uaccess_mm.h:180:26: note: in expansion of macro
>> '__get_user'
#define get_user(x, ptr) __get_user(x, ptr)
^~
drivers/scsi/sg.c:449:14: note: in expansion of macro 'get_user'
retval = get_user(req_pack_id,
^~~~
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/m68k/include/asm/uaccess_mm.h:126:12: note: in definition of macro
'__get_user_asm'
: "m" (*(ptr)), "i" (err));\
^~~
>> arch/m68k/include/asm/uaccess_mm.h:180:26: note: in expansion of macro
>> '__get_user'
#define get_user(x, ptr) __get_user(x, ptr)
^~
drivers/scsi/sg.c:449:14: note: in expansion of macro 'get_user'
retval = get_user(req_pack_id,
^~~~
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/m68k/include/asm/uaccess_mm.h:127:26: note: in definition of macro
'__get_user_asm'
(x) = (__force typeof(*(ptr)))(__force unsigned long)__gu_val; \
^~~
>> arch/m68k/include/asm/uaccess_mm.h:180:26: note: in expansion of macro
>> '__get_user'
#define get_user(x, ptr) __get_user(x, ptr)
^~
drivers/scsi/sg.c:449:14: note: in expansion of macro 'get_user'
retval = get_user(req_pack_id,
^~~~
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
&((sg_io_hdr_t *)buf->pack_id));
^
arch/m68k/include/asm/uaccess_mm.h:126:12: note: in definition of macro
'__get_user_asm'
: "m" (*(ptr)), "i" (err));\
^~~
>> arch/m68k/include/asm/uaccess_mm.h:180:26: note: in expansion of macro
>> '__get_user'
#define get_user(x, ptr) __get_user(x, ptr)
^~
drivers/scsi/sg.c:449:14: note: in expansion of macro 'get_user'
retval = get_user(req_pack_id,
^~~~
drivers/scsi/sg.c:450:27: error: request for member 'pack_id' in something
not a structure or union
Re: [PATCH 00/20] drop useless LIST_HEAD
there was discussion about this just some days ago. CC 4-5 lists is more than enough On 23/12/2018, Julia Lawall wrote: > > > On Sun, 23 Dec 2018, Tom Psyborg wrote: > >> Why do you CC this to so many lists? > > Because the different files are in different subsystems. The cover letter > goes to a list for each file, or to a person if there is no list. The > patches go to the people and lists associated with the affected files. > > julia > >> >> On 23/12/2018, Julia Lawall wrote: >> > Drop LIST_HEAD where the variable it declares is never used. >> > >> > --- >> > >> > drivers/dma/at_hdmac.c|5 - >> > drivers/dma/dw/core.c |1 - >> > drivers/dma/pl330.c |1 - >> > drivers/dma/sa11x0-dma.c |2 -- >> > drivers/dma/st_fdma.c |3 --- >> > drivers/infiniband/ulp/ipoib/ipoib_ib.c |1 - >> > drivers/net/ethernet/mellanox/mlx4/resource_tracker.c |5 - >> > drivers/net/ethernet/mellanox/mlx5/core/en_tc.c |3 --- >> > drivers/net/ethernet/mellanox/mlxsw/spectrum.c|1 - >> > drivers/net/wireless/st/cw1200/queue.c|1 - >> > drivers/scsi/lpfc/lpfc_nvme.c |2 -- >> > drivers/scsi/lpfc/lpfc_scsi.c |2 -- >> > drivers/scsi/lpfc/lpfc_sli.c |1 - >> > drivers/scsi/qla2xxx/qla_init.c |1 - >> > drivers/xen/xenbus/xenbus_dev_frontend.c |2 -- >> > fs/btrfs/relocation.c |1 - >> > fs/nfs/nfs4client.c |1 - >> > fs/nfsd/nfs4layouts.c |1 - >> > fs/xfs/xfs_buf.c |1 - >> > fs/xfs/xfs_fsops.c|1 - >> > 20 files changed, 36 deletions(-) >> > >> >
[PATCH] scsi: avoid a double-fetch and a redundant copy
What we need is only "pack_id", so do not create a heap object or copy
the whole object in. The fix efficiently copies "pack_id" only.
Signed-off-by: Kangjie Lu
---
drivers/scsi/sg.c | 4 ++--
kernel/sched/core.c | 18 --
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 41774e4f9508..75842ceabc7f 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -446,8 +446,8 @@ sg_read(struct file *filp, char __user *buf, size_t count,
loff_t * ppos)
}
if (old_hdr->reply_len < 0) {
if (count >= SZ_SG_IO_HDR) {
- retval = get_user(req_pack_id,
- &((sg_io_hdr_t *)buf->pack_id));
+ retval =
+ get_user(req_pack_id, &(sg_io_hdr_t
__user *)buf->pack_id);
if (retval) {
retval = -EFAULT;
goto free_old_hdr;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 6fedf3a98581..0a55bdce9a42 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4447,7 +4447,7 @@ do_sched_setscheduler(pid_t pid, int policy, struct
sched_param __user *param)
*/
static int sched_copy_attr(struct sched_attr __user *uattr, struct sched_attr
*attr)
{
- u32 size;
+ u32 size, size_cp;
int ret;
if (!access_ok(VERIFY_WRITE, uattr, SCHED_ATTR_SIZE_VER0))
@@ -4460,15 +4460,17 @@ static int sched_copy_attr(struct sched_attr __user
*uattr, struct sched_attr *a
if (ret)
return ret;
+ size_cp = size;
+
/* Bail out on silly large: */
if (size > PAGE_SIZE)
goto err_size;
/* ABI compatibility quirk: */
if (!size)
- size = SCHED_ATTR_SIZE_VER0;
+ size_cp = SCHED_ATTR_SIZE_VER0;
- if (size < SCHED_ATTR_SIZE_VER0)
+ else if (size < SCHED_ATTR_SIZE_VER0)
goto err_size;
/*
@@ -4483,7 +4485,7 @@ static int sched_copy_attr(struct sched_attr __user
*uattr, struct sched_attr *a
unsigned char val;
addr = (void __user *)uattr + sizeof(*attr);
- end = (void __user *)uattr + size;
+ end = (void __user *)uattr + size_cp;
for (; addr < end; addr++) {
ret = get_user(val, addr);
@@ -4492,13 +4494,17 @@ static int sched_copy_attr(struct sched_attr __user
*uattr, struct sched_attr *a
if (val)
goto err_size;
}
- size = sizeof(*attr);
+ size_cp = sizeof(*attr);
}
- ret = copy_from_user(attr, uattr, size);
+ ret = copy_from_user(attr, uattr, size_cp);
if (ret)
return -EFAULT;
+ /* Sanity check if size was changed in user space */
+ if (attr->size != size)
+ return -EINVAL;
+
/*
* XXX: Do we want to be lenient like existing syscalls; or do we want
* to be strict and return an error on out-of-bounds values?
--
2.17.2 (Apple Git-113)
[PATCH v8 03/25] m68k/atari: Replace nvram_{read,write}_byte with arch_nvram_ops
By implementing an arch_nvram_ops struct, any platform can re-use the
drivers/char/nvram.c module without needing any arch-specific code
in that module. Atari does so here.
Atari has one user of nvram_check_checksum() whereas the other "CMOS"
platforms don't use that function at all. Replace this
validate-checksum-and-read-byte sequence with the equivalent
rtc_nvram_ops.read() call and remove the now unused functions.
Signed-off-by: Finn Thain
Tested-by: Christian T. Steigies
Acked-by: Geert Uytterhoeven
---
The advantage of the new ops struct over the old global nvram_* functions
is that the misc device module can be shared by different platforms
without requiring every platform to implement every nvram_* function.
E.g. only RTC "CMOS" NVRAMs have a checksum for the entire NVRAM
and only PowerPC platforms have a "sync" ioctl.
---
arch/m68k/atari/nvram.c | 89 ---
drivers/scsi/atari_scsi.c | 8 ++--
include/linux/nvram.h | 9
3 files changed, 70 insertions(+), 36 deletions(-)
diff --git a/arch/m68k/atari/nvram.c b/arch/m68k/atari/nvram.c
index 3e620ee955ba..bafc9dc32830 100644
--- a/arch/m68k/atari/nvram.c
+++ b/arch/m68k/atari/nvram.c
@@ -39,33 +39,12 @@ unsigned char __nvram_read_byte(int i)
return CMOS_READ(NVRAM_FIRST_BYTE + i);
}
-unsigned char nvram_read_byte(int i)
-{
- unsigned long flags;
- unsigned char c;
-
- spin_lock_irqsave(&rtc_lock, flags);
- c = __nvram_read_byte(i);
- spin_unlock_irqrestore(&rtc_lock, flags);
- return c;
-}
-EXPORT_SYMBOL(nvram_read_byte);
-
/* This races nicely with trying to read with checksum checking */
void __nvram_write_byte(unsigned char c, int i)
{
CMOS_WRITE(c, NVRAM_FIRST_BYTE + i);
}
-void nvram_write_byte(unsigned char c, int i)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&rtc_lock, flags);
- __nvram_write_byte(c, i);
- spin_unlock_irqrestore(&rtc_lock, flags);
-}
-
/* On Ataris, the checksum is over all bytes except the checksum bytes
* themselves; these are at the very end.
*/
@@ -84,18 +63,6 @@ int __nvram_check_checksum(void)
(__nvram_read_byte(ATARI_CKS_LOC + 1) == (sum & 0xff));
}
-int nvram_check_checksum(void)
-{
- unsigned long flags;
- int rv;
-
- spin_lock_irqsave(&rtc_lock, flags);
- rv = __nvram_check_checksum();
- spin_unlock_irqrestore(&rtc_lock, flags);
- return rv;
-}
-EXPORT_SYMBOL(nvram_check_checksum);
-
static void __nvram_set_checksum(void)
{
int i;
@@ -107,6 +74,62 @@ static void __nvram_set_checksum(void)
__nvram_write_byte(sum, ATARI_CKS_LOC + 1);
}
+static ssize_t nvram_read(char *buf, size_t count, loff_t *ppos)
+{
+ char *p = buf;
+ loff_t i;
+
+ spin_lock_irq(&rtc_lock);
+
+ if (!__nvram_check_checksum()) {
+ spin_unlock_irq(&rtc_lock);
+ return -EIO;
+ }
+
+ for (i = *ppos; count > 0 && i < NVRAM_BYTES; --count, ++i, ++p)
+ *p = __nvram_read_byte(i);
+
+ spin_unlock_irq(&rtc_lock);
+
+ *ppos = i;
+ return p - buf;
+}
+
+static ssize_t nvram_write(char *buf, size_t count, loff_t *ppos)
+{
+ char *p = buf;
+ loff_t i;
+
+ spin_lock_irq(&rtc_lock);
+
+ if (!__nvram_check_checksum()) {
+ spin_unlock_irq(&rtc_lock);
+ return -EIO;
+ }
+
+ for (i = *ppos; count > 0 && i < NVRAM_BYTES; --count, ++i, ++p)
+ __nvram_write_byte(*p, i);
+
+ __nvram_set_checksum();
+
+ spin_unlock_irq(&rtc_lock);
+
+ *ppos = i;
+ return p - buf;
+}
+
+static ssize_t nvram_get_size(void)
+{
+ return NVRAM_BYTES;
+}
+
+const struct nvram_ops arch_nvram_ops = {
+ .read = nvram_read,
+ .write = nvram_write,
+ .get_size = nvram_get_size,
+};
+EXPORT_SYMBOL(arch_nvram_ops);
+
#ifdef CONFIG_PROC_FS
static struct {
unsigned char val;
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 99e5729d910d..f55d13e400da 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -760,13 +760,15 @@ static int __init atari_scsi_probe(struct platform_device
*pdev)
#ifdef CONFIG_NVRAM
else
/* Test if a host id is set in the NVRam */
- if (ATARIHW_PRESENT(TT_CLK) && nvram_check_checksum()) {
- unsigned char b = nvram_read_byte(16);
+ if (ATARIHW_PRESENT(TT_CLK)) {
+ unsigned char b;
+ loff_t offset = 16;
+ ssize_t count = arch_nvram_ops.read(&b, 1, &offset);
/* Arbitration enabled? (for TOS)
* If yes, use configured host ID
*/
- if (b & 0x80)
+ if ((count == 1) && (b & 0x80))
atari_scs
[PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
On powerpc, setting CONFIG_NVRAM=n builds a kernel with no NVRAM support.
Setting CONFIG_NVRAM=m enables the /dev/nvram misc device module without
enabling NVRAM support in drivers. Setting CONFIG_NVRAM=y enables the
misc device (built-in) and also enables NVRAM support in drivers.
m68k shares the valkyriefb driver with powerpc, and since that driver uses
NVRAM, it is affected by CONFIG_ATARI_SCSI, because of the use of
"select NVRAM".
Adopt the powerpc convention on m68k to avoid surprises.
Signed-off-by: Finn Thain
Tested-by: Christian T. Steigies
---
This patch temporarily disables CONFIG_NVRAM on Atari, to prevent build
failures when bisecting the rest of this patch series. It gets enabled
again with the introduction of CONFIG_HAVE_ARCH_NVRAM_OPS, once the
nvram_* global functions have been moved to an ops struct.
---
drivers/char/Kconfig | 5 +
drivers/scsi/Kconfig | 6 +++---
drivers/scsi/atari_scsi.c | 7 ---
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 9d03b2ff5df6..5b54595dfe30 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -236,7 +236,7 @@ source "drivers/char/hw_random/Kconfig"
config NVRAM
tristate "/dev/nvram support"
- depends on ATARI || X86 || GENERIC_NVRAM
+ depends on X86 || GENERIC_NVRAM
---help---
If you say Y here and create a character special file /dev/nvram
with major number 10 and minor number 144 using mknod ("man mknod"),
@@ -254,9 +254,6 @@ config NVRAM
should NEVER idly tamper with it. See Ralf Brown's interrupt list
for a guide to the use of CMOS bytes by your BIOS.
- On Atari machines, /dev/nvram is always configured and does not need
- to be selected.
-
To compile this driver as a module, choose M here: the
module will be called nvram.
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 640cd1b31a18..924eb69e7fc4 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -1381,14 +1381,14 @@ config ATARI_SCSI
tristate "Atari native SCSI support"
depends on ATARI && SCSI
select SCSI_SPI_ATTRS
- select NVRAM
---help---
If you have an Atari with built-in NCR5380 SCSI controller (TT,
Falcon, ...) say Y to get it supported. Of course also, if you have
a compatible SCSI controller (e.g. for Medusa).
- To compile this driver as a module, choose M here: the
- module will be called atari_scsi.
+ To compile this driver as a module, choose M here: the module will
+ be called atari_scsi. If you also enable NVRAM support, the SCSI
+ host's ID is taken from the setting in TT RTC NVRAM.
This driver supports both styles of NCR integration into the
system: the TT style (separate DMA), and the Falcon style (via
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 89f5154c40b6..99e5729d910d 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct platform_device
*pdev)
if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
- if (setup_hostid >= 0) {
+ if (setup_hostid >= 0)
atari_scsi_template.this_id = setup_hostid & 7;
- } else {
+#ifdef CONFIG_NVRAM
+ else
/* Test if a host id is set in the NVRam */
if (ATARIHW_PRESENT(TT_CLK) && nvram_check_checksum()) {
unsigned char b = nvram_read_byte(16);
@@ -768,7 +769,7 @@ static int __init atari_scsi_probe(struct platform_device
*pdev)
if (b & 0x80)
atari_scsi_template.this_id = b & 7;
}
- }
+#endif
/* If running on a Falcon and if there's TT-Ram (i.e., more than one
* memory block, since there's always ST-Ram in a Falcon), then
--
2.19.2
Re: [PATCH] scsi: fix a double-fetch bug in sg_write
On 2018-12-25 3:24 p.m., Kangjie Lu wrote: "opcode" has been copied in from user space and checked. We should not copy it in again, which may have been modified by malicous multi-threading user programs through race conditions. The fix uses the opcode fetched in the first copy. Signed-off-by: Kangjie Lu Acked-by: Douglas Gilbert Also applied to my sg v4 driver code. The v1 and v2 interfaces (based on struct sg_header) did not provide a command length field. The sg driver needed to read the first byte of the command (the "opcode") to determine the full command's length prior to actually reading it in full. Hard to think of an example of an exploit based on this double read. --- drivers/scsi/sg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 4dacbfffd113..41774e4f9508 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -686,7 +686,8 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos) hp->flags = input_size; /* structure abuse ... */ hp->pack_id = old_hdr.pack_id; hp->usr_ptr = NULL; - if (__copy_from_user(cmnd, buf, cmd_size)) + cmnd[0] = opcode; + if (__copy_from_user(cmnd + 1, buf + 1, cmd_size - 1)) return -EFAULT; /* * SG_DXFER_TO_FROM_DEV is functionally equivalent to SG_DXFER_FROM_DEV,
[PATCH] scsi: associate bio write hint with WRITE CDB
In SPC-3, WRITE(10)/(16) support grouping function.
Let's associate bio write hint with group number for
enabling StreamID or Turbo Write feature.
Signed-off-by: Randall Huang
---
drivers/scsi/sd.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 4b49cb67617e..28bfa9ed2b54 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1201,7 +1201,12 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd
*SCpnt)
SCpnt->cmnd[11] = (unsigned char) (this_count >> 16) & 0xff;
SCpnt->cmnd[12] = (unsigned char) (this_count >> 8) & 0xff;
SCpnt->cmnd[13] = (unsigned char) this_count & 0xff;
- SCpnt->cmnd[14] = SCpnt->cmnd[15] = 0;
+ if (rq_data_dir(rq) == WRITE) {
+ SCpnt->cmnd[14] = rq->bio->bi_write_hint & 0x3f;
+ } else {
+ SCpnt->cmnd[14] = 0;
+ }
+ SCpnt->cmnd[15] = 0;
} else if ((this_count > 0xff) || (block > 0x1f) ||
scsi_device_protection(SCpnt->device) ||
SCpnt->device->use_10_for_rw) {
@@ -1211,9 +1216,14 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd
*SCpnt)
SCpnt->cmnd[3] = (unsigned char) (block >> 16) & 0xff;
SCpnt->cmnd[4] = (unsigned char) (block >> 8) & 0xff;
SCpnt->cmnd[5] = (unsigned char) block & 0xff;
- SCpnt->cmnd[6] = SCpnt->cmnd[9] = 0;
+ if (rq_data_dir(rq) == WRITE) {
+ SCpnt->cmnd[6] = rq->bio->bi_write_hint & 0x1f;
+ } else {
+ SCpnt->cmnd[6] = 0;
+ }
SCpnt->cmnd[7] = (unsigned char) (this_count >> 8) & 0xff;
SCpnt->cmnd[8] = (unsigned char) this_count & 0xff;
+ SCpnt->cmnd[9] = 0;
} else {
if (unlikely(rq->cmd_flags & REQ_FUA)) {
/*
--
2.20.1.415.g653613c723-goog
[PATCH] scsi: associate bio write hint with WRITE CDB
In SPC-3, WRITE(10)/(16) support grouping function.
Let's associate bio write hint with group number for
enabling StreamID or Turbo Write feature.
Signed-off-by: Randall Huang
---
drivers/scsi/sd.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 4b49cb67617e..28bfa9ed2b54 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1201,7 +1201,12 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd
*SCpnt)
SCpnt->cmnd[11] = (unsigned char) (this_count >> 16) & 0xff;
SCpnt->cmnd[12] = (unsigned char) (this_count >> 8) & 0xff;
SCpnt->cmnd[13] = (unsigned char) this_count & 0xff;
- SCpnt->cmnd[14] = SCpnt->cmnd[15] = 0;
+ if (rq_data_dir(rq) == WRITE) {
+ SCpnt->cmnd[14] = rq->bio->bi_write_hint & 0x3f;
+ } else {
+ SCpnt->cmnd[14] = 0;
+ }
+ SCpnt->cmnd[15] = 0;
} else if ((this_count > 0xff) || (block > 0x1f) ||
scsi_device_protection(SCpnt->device) ||
SCpnt->device->use_10_for_rw) {
@@ -1211,9 +1216,14 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd
*SCpnt)
SCpnt->cmnd[3] = (unsigned char) (block >> 16) & 0xff;
SCpnt->cmnd[4] = (unsigned char) (block >> 8) & 0xff;
SCpnt->cmnd[5] = (unsigned char) block & 0xff;
- SCpnt->cmnd[6] = SCpnt->cmnd[9] = 0;
+ if (rq_data_dir(rq) == WRITE) {
+ SCpnt->cmnd[6] = rq->bio->bi_write_hint & 0x1f;
+ } else {
+ SCpnt->cmnd[6] = 0;
+ }
SCpnt->cmnd[7] = (unsigned char) (this_count >> 8) & 0xff;
SCpnt->cmnd[8] = (unsigned char) this_count & 0xff;
+ SCpnt->cmnd[9] = 0;
} else {
if (unlikely(rq->cmd_flags & REQ_FUA)) {
/*
--
2.20.1.415.g653613c723-goog
[PATCH] scsi: aacraid: add a check for aac_fib_send
aac_fib_send could fail, so add a check to its return value: If it
fails, issue an error message.
Signed-off-by: Kangjie Lu
---
drivers/scsi/aacraid/dpcsup.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/scsi/aacraid/dpcsup.c b/drivers/scsi/aacraid/dpcsup.c
index ddc69738375f..0fbdc0dbf9d8 100644
--- a/drivers/scsi/aacraid/dpcsup.c
+++ b/drivers/scsi/aacraid/dpcsup.c
@@ -271,6 +271,8 @@ static void aac_aif_callback(void *context, struct fib *
fibptr)
FsaNormal,
0, 1,
(fib_callback)aac_aif_callback, fibctx);
+ if (status)
+ pr_err("failure in aac_fib_send: %d\n", status);
}
--
2.17.2 (Apple Git-113)
Re: [PATCH] scsi: avoid a double-fetch and a redundant copy
On 2018-12-25 3:15 p.m., Kangjie Lu wrote:
What we need is only "pack_id", so do not create a heap object or copy
the whole object in. The fix efficiently copies "pack_id" only.
Now this looks like a worthwhile optimization, in some pretty tricky
code. I can't see a security angle in it. Did you test it?
Well the code as presented doesn't compile and the management takes a
dim view of that.
Signed-off-by: Kangjie Lu
---
drivers/scsi/sg.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index c6ad00703c5b..4dacbfffd113 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -446,16 +446,8 @@ sg_read(struct file *filp, char __user *buf, size_t count,
loff_t * ppos)
}
if (old_hdr->reply_len < 0) {
if (count >= SZ_SG_IO_HDR) {
- 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 =__copy_from_user
- (new_hdr, buf, SZ_SG_IO_HDR);
- req_pack_id = new_hdr->pack_id;
- kfree(new_hdr);
+ retval = get_user(req_pack_id,
+ &((sg_io_hdr_t *)buf->pack_id));
The '->' binds higher then the cast and since buf is a 'char *' it doesn't
have a member called pack_id .
Hopefully your drive to remove redundancy went a little too far and removed
the required (but missing) parentheses binding the cast to 'buf'.
if (retval) {
retval = -EFAULT;
goto free_old_hdr;
Good work, silly mistake, but its got me thinking, the heap allocation can be
replaced by stack since its short. The code in this area is more tricky in
the v4 driver because I want to specifically exclude the sg_io_v4 (aka v4)
interface being sent through write(2)/read(2). The way to do that is to read
the first 32 bit integer which should be 'S' or v3, 'Q' for v4.
Hmm, just looking further along my mailer I see the kbuild test robot
has picked up the error and you have presented another patch which also
won't compile. Please stop doing that; apply your patch to kernel source
and compile it _before_ sending it to this list.
Doug Gilbert
[PATCH] target: fix a missing check for match_int
When match_int fails, "arg" is left uninitialized and may contain random
value, thus should not be used.
The fix checks if match_int fails, and if so, break.
Signed-off-by: Kangjie Lu
---
drivers/target/target_core_rd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index a6e8106abd6f..3138123143e8 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -573,7 +573,8 @@ static ssize_t rd_set_configfs_dev_params(struct se_device
*dev,
token = match_token(ptr, tokens, args);
switch (token) {
case Opt_rd_pages:
- match_int(args, &arg);
+ if (match_int(args, &arg))
+ break;
rd_dev->rd_page_count = arg;
pr_debug("RAMDISK: Referencing Page"
" Count: %u\n", rd_dev->rd_page_count);
--
2.17.2 (Apple Git-113)
