| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 05 Jul 2011 21:33:57 -0700
From: Stephen Boyd <sboyd@...eaurora.org>
To: Andrew Morton <akpm@...ux-foundation.org>
CC: linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] x86: Implement strict user copy checks for x86_64
On 6/30/2011 12:36 PM, Andrew Morton wrote:
> On Thu, 30 Jun 2011 12:23:56 -0700
> Stephen Boyd <sboyd@...eaurora.org> wrote:
>
>> Care to share the warnings? I'll run a build again and fix any new
>> warnings I find.
I only get one warning
In file included from /local/mnt2/workspace2/android/kernel/arch/x86/include/asm/uaccess.h:572,
from /local/mnt2/workspace2/android/kernel/include/linux/uaccess.h:5,
from /local/mnt2/workspace2/android/kernel/drivers/staging/speakup/devsynth.c:4:
In function 'copy_from_user',
inlined from 'speakup_file_write' at /local/mnt2/workspace2/android/kernel/drivers/staging/speakup/devsynth.c:28:
/local/mnt2/workspace2/android/kernel/arch/x86/include/asm/uaccess_64.h:64: warning: call to 'copy_from_user_overflow' declared with attribute warning: copy_from_user() buffer size is not provably correct
But that's probably due to my compiler's age more than anything else
(and it seems you fixed it last Friday).
I did notice that I need to do an obj-y instead of a lib-y in the
Makefile. Can you please squash this into the patch titled
"consolidate-config_debug_strict_user_copy_checks.patch"?
----8<----->8------
diff --git a/lib/Makefile b/lib/Makefile
index 785f9b0..9ca779a 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -14,7 +14,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
proportions.o prio_heap.o ratelimit.o show_mem.o \
is_single_threaded.o plist.o decompress.o find_next_bit.o
-lib-$(CONFIG_ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS) += usercopy.o
+obj-$(CONFIG_ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS) += usercopy.o
lib-$(CONFIG_MMU) += ioremap.o
lib-$(CONFIG_SMP) += cpumask.o
> In file included from /usr/src/devel/arch/x86/include/asm/uaccess.h:572,
> from include/linux/uaccess.h:5,
> from include/linux/highmem.h:7,
> from include/linux/pagemap.h:10,
> from include/linux/mempolicy.h:70,
> from mm/mempolicy.c:68:
>
[snip]
> In function 'copy_from_user',
> inlined from '__tun_chr_ioctl' at drivers/net/tun.c:1246:
> /usr/src/devel/arch/x86/include/asm/uaccess_64.h:64: warning: call to 'copy_from_user_overflow' declared with attribute warning: copy_from_user() buffer size is not provably correct
>
Does this help at all?
-----8<------->8-------
diff --git a/drivers/gpu/drm/radeon/radeon_state.c b/drivers/gpu/drm/radeon/radeon_state.c
index 92e7ea7..5c8f53c 100644
--- a/drivers/gpu/drm/radeon/radeon_state.c
+++ b/drivers/gpu/drm/radeon/radeon_state.c
@@ -2169,7 +2169,7 @@ static int radeon_cp_clear(struct drm_device *dev, void *data, struct drm_file *
sarea_priv->nbox = RADEON_NR_SAREA_CLIPRECTS;
if (DRM_COPY_FROM_USER(&depth_boxes, clear->depth_boxes,
- sarea_priv->nbox * sizeof(depth_boxes[0])))
+ (size_t)sarea_priv->nbox * sizeof(depth_boxes[0])))
return -EFAULT;
radeon_cp_dispatch_clear(dev, file_priv->master, clear, depth_boxes);
diff --git a/drivers/message/i2o/i2o_config.c b/drivers/message/i2o/i2o_config.c
index 098de2b..4dcdc3d 100644
--- a/drivers/message/i2o/i2o_config.c
+++ b/drivers/message/i2o/i2o_config.c
@@ -680,6 +680,10 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd,
}
size = size >> 16;
size *= 4;
+ if (size > sizeof(rmsg)) {
+ rcode = -EINVAL;
+ goto sg_list_cleanup;
+ }
/* Copy in the user's I2O command */
if (copy_from_user(rmsg, user_msg, size)) {
rcode = -EFAULT;
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 909ed9e..ce76d0c 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -2367,16 +2367,15 @@ static ssize_t
sg_proc_write_adio(struct file *filp, const char __user *buffer,
size_t count, loff_t *off)
{
- int num;
- char buff[11];
+ int err;
+ unsigned long num;
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
- num = (count < 10) ? count : 10;
- if (copy_from_user(buff, buffer, num))
- return -EFAULT;
- buff[num] = '\0';
- sg_allow_dio = simple_strtoul(buff, NULL, 10) ? 1 : 0;
+ err = kstrtoul_from_user(buffer, count, 0, &num);
+ if (err)
+ return err;
+ sg_allow_dio = num ? 1 : 0;
return count;
}
@@ -2389,17 +2388,15 @@ static ssize_t
sg_proc_write_dressz(struct file *filp, const char __user *buffer,
size_t count, loff_t *off)
{
- int num;
+ int err;
unsigned long k = ULONG_MAX;
- char buff[11];
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
- num = (count < 10) ? count : 10;
- if (copy_from_user(buff, buffer, num))
- return -EFAULT;
- buff[num] = '\0';
- k = simple_strtoul(buff, NULL, 10);
+
+ err = kstrtoul_from_user(buffer, count, 0, &k);
+ if (err)
+ return err;
if (k <= 1048576) { /* limit "big buff" to 1 MB */
sg_big_buff = k;
return count;
diff --git a/lib/Makefile b/lib/Makefile
index 785f9b0..9ca779a 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -14,7 +14,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
proportions.o prio_heap.o ratelimit.o show_mem.o \
is_single_threaded.o plist.o decompress.o find_next_bit.o
-lib-$(CONFIG_ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS) += usercopy.o
+obj-$(CONFIG_ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS) += usercopy.o
lib-$(CONFIG_MMU) += ioremap.o
lib-$(CONFIG_SMP) += cpumask.o
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index e7fb9d2..e9d4987 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1405,8 +1405,14 @@ asmlinkage long compat_sys_get_mempolicy(int __user *policy,
nr_bits = min_t(unsigned long, maxnode-1, MAX_NUMNODES);
alloc_size = ALIGN(nr_bits, BITS_PER_LONG) / 8;
- if (nmask)
+ if (alloc_size > sizeof(bm))
+ return -EINVAL;
+
+ if (nmask) {
nm = compat_alloc_user_space(alloc_size);
+ if (!nm)
+ return -ENOMEM;
+ }
err = sys_get_mempolicy(policy, nm, nr_bits+1, addr, flags);
diff --git a/net/compat.c b/net/compat.c
index c578d93..b769233 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -789,6 +789,8 @@ asmlinkage long compat_sys_socketcall(int call, u32 __user *args)
if (call < SYS_SOCKET || call > SYS_SENDMMSG)
return -EINVAL;
+ if (nas[call] > sizeof(a))
+ return -EINVAL;
if (copy_from_user(a, args, nas[call]))
return -EFAULT;
a0 = a[0];
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index f76079c..2f4ea06 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1382,6 +1382,8 @@ static ssize_t pktgen_if_write(struct file *file,
len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_min) - 1);
if (len < 0)
return len;
+ if (len > sizeof(buf))
+ return -EINVAL;
if (copy_from_user(buf, &user_buffer[i], len))
return -EFAULT;
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists