2011-05-12 23:50:22

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 0/9] strict user copy checks on x86_64

It turns out that strict user copy checks (also known as
CONFIG_DEBUG_STRICT_USER_COPY_CHECKS) isn't actually implemented
on x86_64 and thus we aren't catching potential security holes
at compile time.

This series adds support for strict user copy checks on x86_64
and silences all the benign warnings in the x86_64 allyesconfig.

The final patch consolidates the config option as its duplicated
across mutliple arches. I don't know what tree this series should
go through so I tried to send the individual driver patches to the
respective maintainers.

Stephen Boyd (9):
iwlegacy: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
iwlwifi: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
[SCSI] lpfc: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
debugfs: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
kprobes: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
Bluetooth: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
ASoC: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
x86: Implement strict user copy checks for x86_64
Consolidate CONFIG_DEBUG_STRICT_USER_COPY_CHECKS

arch/parisc/Kconfig | 1 +
arch/parisc/Kconfig.debug | 14 --------------
arch/s390/Kconfig | 1 +
arch/s390/Kconfig.debug | 14 --------------
arch/s390/lib/Makefile | 1 -
arch/s390/lib/usercopy.c | 8 --------
arch/sparc/lib/Makefile | 1 -
arch/sparc/lib/usercopy.c | 8 --------
arch/tile/Kconfig | 8 +-------
arch/tile/include/asm/uaccess.h | 7 ++++++-
arch/tile/lib/uaccess.c | 8 --------
arch/x86/Kconfig | 1 +
arch/x86/Kconfig.debug | 14 --------------
arch/x86/include/asm/uaccess_64.h | 12 +++++++++---
arch/x86/lib/usercopy_32.c | 6 ------
drivers/net/wireless/iwlegacy/iwl-4965-rs.c | 2 +-
drivers/net/wireless/iwlwifi/iwl-agn-rs.c | 2 +-
drivers/scsi/lpfc/lpfc_debugfs.c | 3 ++-
fs/debugfs/file.c | 2 +-
kernel/kprobes.c | 2 +-
lib/Kconfig.debug | 18 ++++++++++++++++++
lib/Makefile | 1 +
lib/usercopy.c | 8 ++++++++
net/bluetooth/rfcomm/sock.c | 3 ++-
sound/soc/soc-core.c | 2 +-
25 files changed, 55 insertions(+), 92 deletions(-)
delete mode 100644 arch/s390/lib/usercopy.c
delete mode 100644 arch/sparc/lib/usercopy.c
create mode 100644 lib/usercopy.c

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


2011-05-12 23:50:20

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 1/9] iwlegacy: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
warning:

In file included from arch/x86/include/asm/uaccess.h:573,
from include/net/checksum.h:25,
from include/linux/skbuff.h:28,
from drivers/net/wireless/iwlegacy/iwl-4965-rs.c:28:
In function 'copy_from_user',
inlined from 'iwl4965_rs_sta_dbgfs_scale_table_write' at
drivers/net/wireless/iwlegacy/iwl-4965-rs.c:2616:
arch/x86/include/asm/uaccess_64.h:65:
warning: call to 'copy_from_user_overflow' declared with
attribute warning: copy_from_user() buffer size is not provably
correct

presumably due to buf_size being signed causing GCC to fail to
see that buf_size can't become negative.

Cc: Johannes Berg <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/net/wireless/iwlegacy/iwl-4965-rs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
index 31ac672..cc4751c 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
@@ -2604,7 +2604,7 @@ static ssize_t iwl4965_rs_sta_dbgfs_scale_table_write(struct file *file,
struct iwl_lq_sta *lq_sta = file->private_data;
struct iwl_priv *priv;
char buf[64];
- int buf_size;
+ size_t buf_size;
u32 parsed_rate;
struct iwl_station_priv *sta_priv =
container_of(lq_sta, struct iwl_station_priv, lq_sta);
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-05-12 23:50:27

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 2/9] iwlwifi: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
warning:

In file included from arch/x86/include/asm/uaccess.h:573,
from include/net/checksum.h:25,
from include/linux/skbuff.h:28,
from drivers/net/wireless/iwlwifi/iwl-agn-rs.c:28:
In function 'copy_from_user',
inlined from 'rs_sta_dbgfs_scale_table_write' at
drivers/net/wireless/iwlwifi/iwl-agn-rs.c:3099:
arch/x86/include/asm/uaccess_64.h:65:
warning: call to 'copy_from_user_overflow' declared with
attribute warning: copy_from_user() buffer size is not provably
correct

presumably due to buf_size being signed causing GCC to fail to
see that buf_size can't become negative.

Cc: Wey-Yi Guy <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-agn-rs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rs.c b/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
index d03b473..abcc9f2 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
@@ -3087,7 +3087,7 @@ static ssize_t rs_sta_dbgfs_scale_table_write(struct file *file,
struct iwl_lq_sta *lq_sta = file->private_data;
struct iwl_priv *priv;
char buf[64];
- int buf_size;
+ size_t buf_size;
u32 parsed_rate;
struct iwl_station_priv *sta_priv =
container_of(lq_sta, struct iwl_station_priv, lq_sta);
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-05-12 23:50:30

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 3/9] [SCSI] lpfc: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
warning:

In file included from arch/x86/include/asm/uaccess.h:573,
from include/linux/uaccess.h:5,
from include/linux/highmem.h:7,
from include/linux/pagemap.h:10,
from include/linux/blkdev.h:12,
from drivers/scsi/lpfc/lpfc_debugfs.c:21:
In function 'copy_from_user':
arch/x86/include/asm/uaccess_64.h:65:
warning: call to 'copy_from_user_overflow' declared with
attribute warning: copy_from_user() buffer size is not provably
correct

presumably due to buf_size being signed causing GCC to fail to
see that buf_size can't become negative.

Cc: James Smart <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/scsi/lpfc/lpfc_debugfs.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 3d96774..0af53a6 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -1305,7 +1305,8 @@ static int lpfc_idiag_cmd_get(const char __user *buf, size_t nbytes,
{
char mybuf[64];
char *pbuf, *step_str;
- int bsize, i;
+ int i;
+ size_t bsize;

/* Protect copy from user */
if (!access_ok(VERIFY_READ, buf, nbytes))
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-05-12 23:50:32

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 4/9] debugfs: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
warning:

In file included from arch/x86/include/asm/uaccess.h:573,
from include/linux/uaccess.h:5,
from include/linux/highmem.h:7,
from include/linux/pagemap.h:10,
from fs/debugfs/file.c:18:
In function 'copy_from_user',
inlined from 'write_file_bool' at fs/debugfs/file.c:435:
arch/x86/include/asm/uaccess_64.h:65: warning: call to
'copy_from_user_overflow' declared with attribute warning:
copy_from_user() buffer size is not provably correct

presumably due to buf_size being signed causing GCC to fail to
see that buf_size can't become negative.

Signed-off-by: Stephen Boyd <[email protected]>
---
fs/debugfs/file.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 89d394d..7ead5b8 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -428,7 +428,7 @@ static ssize_t write_file_bool(struct file *file, const char __user *user_buf,
size_t count, loff_t *ppos)
{
char buf[32];
- int buf_size;
+ size_t buf_size;
u32 *val = file->private_data;

buf_size = min(count, (sizeof(buf)-1));
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-05-12 23:50:24

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 5/9] kprobes: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
warning:

In file included from arch/x86/include/asm/uaccess.h:573,
from kernel/kprobes.c:55:
In function 'copy_from_user',
inlined from 'write_enabled_file_bool' at
kernel/kprobes.c:2191:
arch/x86/include/asm/uaccess_64.h:65:
warning: call to 'copy_from_user_overflow' declared with
attribute warning: copy_from_user() buffer size is not provably
correct

presumably due to buf_size being signed causing GCC to fail to
see that buf_size can't become negative.

Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Anil S Keshavamurthy <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
kernel/kprobes.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 7798181..1938187 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -2185,7 +2185,7 @@ static ssize_t write_enabled_file_bool(struct file *file,
const char __user *user_buf, size_t count, loff_t *ppos)
{
char buf[32];
- int buf_size;
+ size_t buf_size;

buf_size = min(count, (sizeof(buf)-1));
if (copy_from_user(buf, user_buf, buf_size))
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-05-12 23:50:28

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 6/9] Bluetooth: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
warning:

In function 'copy_from_user',
inlined from 'rfcomm_sock_setsockopt' at
net/bluetooth/rfcomm/sock.c:705:
arch/x86/include/asm/uaccess_64.h:65:
warning: call to 'copy_from_user_overflow' declared with
attribute warning: copy_from_user() buffer size is not provably
correct

presumably due to buf_size being signed causing GCC to fail to
see that buf_size can't become negative.

Cc: Marcel Holtmann <[email protected]>
Cc: Gustavo F. Padovan <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 66cc1f0..0698b37 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -679,7 +679,8 @@ static int rfcomm_sock_setsockopt(struct socket *sock, int level, int optname, c
{
struct sock *sk = sock->sk;
struct bt_security sec;
- int len, err = 0;
+ int err = 0;
+ size_t len;
u32 opt;

BT_DBG("sk %p", sk);
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-05-12 23:50:33

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 7/9] ASoC: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
warning:

In file included from arch/x86/include/asm/uaccess.h:573,
from include/linux/poll.h:14,
from include/sound/pcm.h:29,
from include/sound/ac97_codec.h:31,
from sound/soc/soc-core.c:34:
In function 'copy_from_user',
inlined from 'codec_reg_write_file' at
sound/soc/soc-core.c:252:
arch/x86/include/asm/uaccess_64.h:65:
warning: call to 'copy_from_user_overflow' declared with
attribute warning: copy_from_user() buffer size is not provably
correct

presumably due to buf_size being signed causing GCC to fail to
see that buf_size can't become negative.

Cc: Liam Girdwood <[email protected]>
Cc: Mark Brown <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
sound/soc/soc-core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index d8562ce..e8a0f64 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -242,7 +242,7 @@ static ssize_t codec_reg_write_file(struct file *file,
const char __user *user_buf, size_t count, loff_t *ppos)
{
char buf[32];
- int buf_size;
+ size_t buf_size;
char *start = buf;
unsigned long reg, value;
int step = 1;
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-05-12 23:50:34

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 8/9] x86: Implement strict user copy checks for x86_64

Strict user copy checks are only really supported on x86_32 even
though the config option is selectable on x86_64. Add the
necessary support to the 64 bit code to trigger copy_from_user()
warnings at compile time.

Cc: [email protected]
Signed-off-by: Stephen Boyd <[email protected]>
---
arch/x86/include/asm/uaccess_64.h | 12 +++++++++---
arch/x86/lib/usercopy_64.c | 6 ++++++
2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
index 316708d..904684b 100644
--- a/arch/x86/include/asm/uaccess_64.h
+++ b/arch/x86/include/asm/uaccess_64.h
@@ -44,6 +44,14 @@ _copy_from_user(void *to, const void __user *from, unsigned len);
__must_check unsigned long
copy_in_user(void __user *to, const void __user *from, unsigned len);

+extern void copy_from_user_overflow(void)
+#ifdef CONFIG_DEBUG_STRICT_USER_COPY_CHECKS
+ __compiletime_error("copy_from_user() buffer size is not provably correct")
+#else
+ __compiletime_warning("copy_from_user() buffer size is not provably correct")
+#endif
+;
+
static inline unsigned long __must_check copy_from_user(void *to,
const void __user *from,
unsigned long n)
@@ -53,10 +61,8 @@ static inline unsigned long __must_check copy_from_user(void *to,
might_fault();
if (likely(sz == -1 || sz >= n))
n = _copy_from_user(to, from, n);
-#ifdef CONFIG_DEBUG_VM
else
- WARN(1, "Buffer overflow detected!\n");
-#endif
+ copy_from_user_overflow();
return n;
}

diff --git a/arch/x86/lib/usercopy_64.c b/arch/x86/lib/usercopy_64.c
index b7c2849..d7a5d9a 100644
--- a/arch/x86/lib/usercopy_64.c
+++ b/arch/x86/lib/usercopy_64.c
@@ -181,3 +181,9 @@ copy_user_handle_tail(char *to, char *from, unsigned len, unsigned zerorest)
break;
return len;
}
+
+void copy_from_user_overflow(void)
+{
+ WARN(1, "Buffer overflow detected!\n");
+}
+EXPORT_SYMBOL(copy_from_user_overflow);
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-05-12 23:52:42

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 9/9] Consolidate CONFIG_DEBUG_STRICT_USER_COPY_CHECKS

The help text for this config is duplicated across the x86,
parisc, and s390 Kconfig.debug files. Arnd Bergman noted that the
help text was slightly misleading and should be fixed to state
that enabling this option isn't a problem when using pre 4.4 gcc.

To simplify the rewording, consolidate the text into
lib/Kconfig.debug and modify it there to be more explicit about
when you should say N to this config.

Also, make the text a bit more generic by stating that this
option enables compile time checks so we can cover architectures
which emit warnings vs. ones which emit errors. The details of
how an architecture decided to implement the checks isn't as
important as the concept of compile time checking of
copy_from_user() calls.

While we're doing this, remove all the copy_from_user_overflow()
code that's duplicated many times and place it into lib/ so that
any architecture supporting this option can get the function for
free.

Cc: Arnd Bergmann <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Arjan van de Ven <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Chris Metcalf <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
arch/parisc/Kconfig | 1 +
arch/parisc/Kconfig.debug | 14 --------------
arch/s390/Kconfig | 1 +
arch/s390/Kconfig.debug | 14 --------------
arch/s390/lib/Makefile | 1 -
arch/s390/lib/usercopy.c | 8 --------
arch/sparc/lib/Makefile | 1 -
arch/sparc/lib/usercopy.c | 8 --------
arch/tile/Kconfig | 8 +-------
arch/tile/include/asm/uaccess.h | 7 ++++++-
arch/tile/lib/uaccess.c | 8 --------
arch/x86/Kconfig | 1 +
arch/x86/Kconfig.debug | 14 --------------
arch/x86/lib/usercopy_32.c | 6 ------
arch/x86/lib/usercopy_64.c | 6 ------
lib/Kconfig.debug | 18 ++++++++++++++++++
lib/Makefile | 1 +
lib/usercopy.c | 8 ++++++++
18 files changed, 37 insertions(+), 88 deletions(-)
delete mode 100644 arch/s390/lib/usercopy.c
delete mode 100644 arch/sparc/lib/usercopy.c
create mode 100644 lib/usercopy.c

diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 69ff049..4473f10 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -15,6 +15,7 @@ config PARISC
select HAVE_GENERIC_HARDIRQS
select GENERIC_IRQ_PROBE
select IRQ_PER_CPU
+ select ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS

help
The PA-RISC microprocessor is designed by Hewlett-Packard and used
diff --git a/arch/parisc/Kconfig.debug b/arch/parisc/Kconfig.debug
index 7305ac8..bc989e5 100644
--- a/arch/parisc/Kconfig.debug
+++ b/arch/parisc/Kconfig.debug
@@ -12,18 +12,4 @@ config DEBUG_RODATA
portion of the kernel code won't be covered by a TLB anymore.
If in doubt, say "N".

-config DEBUG_STRICT_USER_COPY_CHECKS
- bool "Strict copy size checks"
- depends on DEBUG_KERNEL && !TRACE_BRANCH_PROFILING
- ---help---
- Enabling this option turns a certain set of sanity checks for user
- copy operations into compile time failures.
-
- The copy_from_user() etc checks are there to help test if there
- are sufficient security checks on the length argument of
- the copy operation, by having gcc prove that the argument is
- within bounds.
-
- If unsure, or if you run an older (pre 4.4) gcc, say N.
-
endmenu
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 2508a6f..196cdc9 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -116,6 +116,7 @@ config S390
select ARCH_INLINE_WRITE_UNLOCK_BH
select ARCH_INLINE_WRITE_UNLOCK_IRQ
select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
+ select ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS

config SCHED_OMIT_FRAME_POINTER
def_bool y
diff --git a/arch/s390/Kconfig.debug b/arch/s390/Kconfig.debug
index d76cef3..aa1796c 100644
--- a/arch/s390/Kconfig.debug
+++ b/arch/s390/Kconfig.debug
@@ -17,20 +17,6 @@ config STRICT_DEVMEM

If you are unsure, say Y.

-config DEBUG_STRICT_USER_COPY_CHECKS
- def_bool n
- prompt "Strict user copy size checks"
- ---help---
- Enabling this option turns a certain set of sanity checks for user
- copy operations into compile time warnings.
-
- The copy_from_user() etc checks are there to help test if there
- are sufficient security checks on the length argument of
- the copy operation, by having gcc prove that the argument is
- within bounds.
-
- If unsure, or if you run an older (pre 4.4) gcc, say N.
-
config DEBUG_SET_MODULE_RONX
def_bool y
depends on MODULES
diff --git a/arch/s390/lib/Makefile b/arch/s390/lib/Makefile
index 761ab8b..97975ec 100644
--- a/arch/s390/lib/Makefile
+++ b/arch/s390/lib/Makefile
@@ -3,7 +3,6 @@
#

lib-y += delay.o string.o uaccess_std.o uaccess_pt.o
-obj-y += usercopy.o
obj-$(CONFIG_32BIT) += div64.o qrnnd.o ucmpdi2.o
lib-$(CONFIG_64BIT) += uaccess_mvcos.o
lib-$(CONFIG_SMP) += spinlock.o
diff --git a/arch/s390/lib/usercopy.c b/arch/s390/lib/usercopy.c
deleted file mode 100644
index 14b363f..0000000
--- a/arch/s390/lib/usercopy.c
+++ /dev/null
@@ -1,8 +0,0 @@
-#include <linux/module.h>
-#include <linux/bug.h>
-
-void copy_from_user_overflow(void)
-{
- WARN(1, "Buffer overflow detected!\n");
-}
-EXPORT_SYMBOL(copy_from_user_overflow);
diff --git a/arch/sparc/lib/Makefile b/arch/sparc/lib/Makefile
index 846d1c4..892e0a9 100644
--- a/arch/sparc/lib/Makefile
+++ b/arch/sparc/lib/Makefile
@@ -44,4 +44,3 @@ obj-y += iomap.o
obj-$(CONFIG_SPARC32) += atomic32.o
obj-y += ksyms.o
obj-$(CONFIG_SPARC64) += PeeCeeI.o
-obj-y += usercopy.o
diff --git a/arch/sparc/lib/usercopy.c b/arch/sparc/lib/usercopy.c
deleted file mode 100644
index 14b363f..0000000
--- a/arch/sparc/lib/usercopy.c
+++ /dev/null
@@ -1,8 +0,0 @@
-#include <linux/module.h>
-#include <linux/bug.h>
-
-void copy_from_user_overflow(void)
-{
- WARN(1, "Buffer overflow detected!\n");
-}
-EXPORT_SYMBOL(copy_from_user_overflow);
diff --git a/arch/tile/Kconfig b/arch/tile/Kconfig
index e32b0c2..8961f43 100644
--- a/arch/tile/Kconfig
+++ b/arch/tile/Kconfig
@@ -12,6 +12,7 @@ config TILE
select GENERIC_IRQ_PROBE
select GENERIC_PENDING_IRQ if SMP
select GENERIC_IRQ_SHOW
+ select ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS

# FIXME: investigate whether we need/want these options.
# select HAVE_IOREMAP_PROT
@@ -96,13 +97,6 @@ config STRICT_DEVMEM
config SMP
def_bool y

-# Allow checking for compile-time determined overflow errors in
-# copy_from_user(). There are still unprovable places in the
-# generic code as of 2.6.34, so this option is not really compatible
-# with -Werror, which is more useful in general.
-config DEBUG_COPY_FROM_USER
- def_bool n
-
config HVC_TILE
select HVC_DRIVER
def_bool y
diff --git a/arch/tile/include/asm/uaccess.h b/arch/tile/include/asm/uaccess.h
index ef34d2c..9a540be 100644
--- a/arch/tile/include/asm/uaccess.h
+++ b/arch/tile/include/asm/uaccess.h
@@ -353,7 +353,12 @@ _copy_from_user(void *to, const void __user *from, unsigned long n)
return n;
}

-#ifdef CONFIG_DEBUG_COPY_FROM_USER
+#ifdef CONFIG_DEBUG_STRICT_USER_COPY_CHECKS
+/*
+ * There are still unprovable places in the generic code as of 2.6.34, so this
+ * option is not really compatible with -Werror, which is more useful in
+ * general.
+ */
extern void copy_from_user_overflow(void)
__compiletime_warning("copy_from_user() size is not provably correct");

diff --git a/arch/tile/lib/uaccess.c b/arch/tile/lib/uaccess.c
index f8d398c..030abe3 100644
--- a/arch/tile/lib/uaccess.c
+++ b/arch/tile/lib/uaccess.c
@@ -22,11 +22,3 @@ int __range_ok(unsigned long addr, unsigned long size)
is_arch_mappable_range(addr, size));
}
EXPORT_SYMBOL(__range_ok);
-
-#ifdef CONFIG_DEBUG_COPY_FROM_USER
-void copy_from_user_overflow(void)
-{
- WARN(1, "Buffer overflow detected!\n");
-}
-EXPORT_SYMBOL(copy_from_user_overflow);
-#endif
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc6c53a..be50c8e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -72,6 +72,7 @@ config X86
select IRQ_FORCED_THREADING
select USE_GENERIC_SMP_HELPERS if SMP
select ARCH_NO_SYSDEV_OPS
+ select ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS

config INSTRUCTION_DECODER
def_bool (KPROBES || PERF_EVENTS)
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 615e188..e3c58f3 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -290,18 +290,4 @@ config OPTIMIZE_INLINING

If unsure, say N.

-config DEBUG_STRICT_USER_COPY_CHECKS
- bool "Strict copy size checks"
- depends on DEBUG_KERNEL && !TRACE_BRANCH_PROFILING
- ---help---
- Enabling this option turns a certain set of sanity checks for user
- copy operations into compile time failures.
-
- The copy_from_user() etc checks are there to help test if there
- are sufficient security checks on the length argument of
- the copy operation, by having gcc prove that the argument is
- within bounds.
-
- If unsure, or if you run an older (pre 4.4) gcc, say N.
-
endmenu
diff --git a/arch/x86/lib/usercopy_32.c b/arch/x86/lib/usercopy_32.c
index e218d5d..8498684 100644
--- a/arch/x86/lib/usercopy_32.c
+++ b/arch/x86/lib/usercopy_32.c
@@ -883,9 +883,3 @@ _copy_from_user(void *to, const void __user *from, unsigned long n)
return n;
}
EXPORT_SYMBOL(_copy_from_user);
-
-void copy_from_user_overflow(void)
-{
- WARN(1, "Buffer overflow detected!\n");
-}
-EXPORT_SYMBOL(copy_from_user_overflow);
diff --git a/arch/x86/lib/usercopy_64.c b/arch/x86/lib/usercopy_64.c
index d7a5d9a..b7c2849 100644
--- a/arch/x86/lib/usercopy_64.c
+++ b/arch/x86/lib/usercopy_64.c
@@ -181,9 +181,3 @@ copy_user_handle_tail(char *to, char *from, unsigned len, unsigned zerorest)
break;
return len;
}
-
-void copy_from_user_overflow(void)
-{
- WARN(1, "Buffer overflow detected!\n");
-}
-EXPORT_SYMBOL(copy_from_user_overflow);
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index c768bcd..3703778 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1096,6 +1096,24 @@ config SYSCTL_SYSCALL_CHECK
to properly maintain and use. This enables checks that help
you to keep things correct.

+config ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS
+ bool
+
+config DEBUG_STRICT_USER_COPY_CHECKS
+ bool "Strict user copy size checks"
+ depends on ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS
+ depends on DEBUG_KERNEL && !TRACE_BRANCH_PROFILING
+ help
+ Enabling this option turns a certain set of sanity checks for user
+ copy operations into compile time failures.
+
+ The copy_from_user() etc checks are there to help test if there
+ are sufficient security checks on the length argument of
+ the copy operation, by having gcc prove that the argument is
+ within bounds.
+
+ If unsure, say N.
+
source mm/Kconfig.debug
source kernel/trace/Kconfig

diff --git a/lib/Makefile b/lib/Makefile
index ef0f285..4c1e445 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -14,6 +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

+lib-$(CONFIG_ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS) += usercopy.o
lib-$(CONFIG_MMU) += ioremap.o
lib-$(CONFIG_SMP) += cpumask.o

diff --git a/lib/usercopy.c b/lib/usercopy.c
new file mode 100644
index 0000000..14b363f
--- /dev/null
+++ b/lib/usercopy.c
@@ -0,0 +1,8 @@
+#include <linux/module.h>
+#include <linux/bug.h>
+
+void copy_from_user_overflow(void)
+{
+ WARN(1, "Buffer overflow detected!\n");
+}
+EXPORT_SYMBOL(copy_from_user_overflow);
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-05-13 00:05:03

by Wey-Yi Guy

[permalink] [raw]
Subject: Re: [PATCH 1/9] iwlegacy: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

On Thu, 2011-05-12 at 16:50 -0700, Stephen Boyd wrote:
> Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
> warning:
>
> In file included from arch/x86/include/asm/uaccess.h:573,
> from include/net/checksum.h:25,
> from include/linux/skbuff.h:28,
> from drivers/net/wireless/iwlegacy/iwl-4965-rs.c:28:
> In function 'copy_from_user',
> inlined from 'iwl4965_rs_sta_dbgfs_scale_table_write' at
> drivers/net/wireless/iwlegacy/iwl-4965-rs.c:2616:
> arch/x86/include/asm/uaccess_64.h:65:
> warning: call to 'copy_from_user_overflow' declared with
> attribute warning: copy_from_user() buffer size is not provably
> correct
>
> presumably due to buf_size being signed causing GCC to fail to
> see that buf_size can't become negative.
>
> Cc: Johannes Berg <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/net/wireless/iwlegacy/iwl-4965-rs.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
Stanislaw Gruszka <[email protected]> is the maintainer for iwlegacy

Thanks
Wey


2011-05-13 00:05:47

by Wey-Yi Guy

[permalink] [raw]
Subject: Re: [PATCH 2/9] iwlwifi: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

On Thu, 2011-05-12 at 16:50 -0700, Stephen Boyd wrote:
> Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
> warning:
>
> In file included from arch/x86/include/asm/uaccess.h:573,
> from include/net/checksum.h:25,
> from include/linux/skbuff.h:28,
> from drivers/net/wireless/iwlwifi/iwl-agn-rs.c:28:
> In function 'copy_from_user',
> inlined from 'rs_sta_dbgfs_scale_table_write' at
> drivers/net/wireless/iwlwifi/iwl-agn-rs.c:3099:
> arch/x86/include/asm/uaccess_64.h:65:
> warning: call to 'copy_from_user_overflow' declared with
> attribute warning: copy_from_user() buffer size is not provably
> correct
>
> presumably due to buf_size being signed causing GCC to fail to
> see that buf_size can't become negative.
>
> Cc: Wey-Yi Guy <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
Signed-off-by: Wey-Yi Guy <[email protected]>
> ---
> drivers/net/wireless/iwlwifi/iwl-agn-rs.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
Thank you

Wey

Subject: Re: [PATCH 5/9] kprobes: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

(2011/05/13 8:50), Stephen Boyd wrote:
> Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
> warning:
>
> In file included from arch/x86/include/asm/uaccess.h:573,
> from kernel/kprobes.c:55:
> In function 'copy_from_user',
> inlined from 'write_enabled_file_bool' at
> kernel/kprobes.c:2191:
> arch/x86/include/asm/uaccess_64.h:65:
> warning: call to 'copy_from_user_overflow' declared with
> attribute warning: copy_from_user() buffer size is not provably
> correct
>
> presumably due to buf_size being signed causing GCC to fail to
> see that buf_size can't become negative.

Good catch! At least, since the "count" is size_t, buf_size
should be size_t too.

Acked-by: Masami Hiramatsu <[email protected]>

Thank you!

>
> Cc: Ananth N Mavinakayanahalli <[email protected]>
> Cc: Anil S Keshavamurthy <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> kernel/kprobes.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 7798181..1938187 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -2185,7 +2185,7 @@ static ssize_t write_enabled_file_bool(struct file *file,
> const char __user *user_buf, size_t count, loff_t *ppos)
> {
> char buf[32];
> - int buf_size;
> + size_t buf_size;
>
> buf_size = min(count, (sizeof(buf)-1));
> if (copy_from_user(buf, user_buf, buf_size))


--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2011-05-13 00:55:34

by Chris Metcalf

[permalink] [raw]
Subject: Re: [PATCH 9/9] Consolidate CONFIG_DEBUG_STRICT_USER_COPY_CHECKS

On 5/12/2011 7:50 PM, Stephen Boyd wrote:
> The help text for this config is duplicated across the x86,
> parisc, and s390 Kconfig.debug files. Arnd Bergman noted that the
> help text was slightly misleading and should be fixed to state
> that enabling this option isn't a problem when using pre 4.4 gcc.
>
> To simplify the rewording, consolidate the text into
> lib/Kconfig.debug and modify it there to be more explicit about
> when you should say N to this config.
>
> Also, make the text a bit more generic by stating that this
> option enables compile time checks so we can cover architectures
> which emit warnings vs. ones which emit errors. The details of
> how an architecture decided to implement the checks isn't as
> important as the concept of compile time checking of
> copy_from_user() calls.
>
> While we're doing this, remove all the copy_from_user_overflow()
> code that's duplicated many times and place it into lib/ so that
> any architecture supporting this option can get the function for
> free.
>
> ---
> arch/tile/Kconfig | 8 +-------
> arch/tile/include/asm/uaccess.h | 7 ++++++-
> arch/tile/lib/uaccess.c | 8 --------

Acked-by: Chris Metcalf <[email protected]>

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com

2011-05-13 07:31:32

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 9/9] Consolidate CONFIG_DEBUG_STRICT_USER_COPY_CHECKS


* Stephen Boyd <[email protected]> wrote:

> The help text for this config is duplicated across the x86,
> parisc, and s390 Kconfig.debug files. Arnd Bergman noted that the
> help text was slightly misleading and should be fixed to state
> that enabling this option isn't a problem when using pre 4.4 gcc.
>
> To simplify the rewording, consolidate the text into
> lib/Kconfig.debug and modify it there to be more explicit about
> when you should say N to this config.
>
> Also, make the text a bit more generic by stating that this
> option enables compile time checks so we can cover architectures
> which emit warnings vs. ones which emit errors. The details of
> how an architecture decided to implement the checks isn't as
> important as the concept of compile time checking of
> copy_from_user() calls.
>
> While we're doing this, remove all the copy_from_user_overflow()
> code that's duplicated many times and place it into lib/ so that
> any architecture supporting this option can get the function for
> free.
>
> Cc: Arnd Bergmann <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Arjan van de Ven <[email protected]>
> Cc: Helge Deller <[email protected]>
> Cc: Heiko Carstens <[email protected]>
> Cc: Stephen Rothwell <[email protected]>
> Cc: Chris Metcalf <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> arch/parisc/Kconfig | 1 +
> arch/parisc/Kconfig.debug | 14 --------------
> arch/s390/Kconfig | 1 +
> arch/s390/Kconfig.debug | 14 --------------
> arch/s390/lib/Makefile | 1 -
> arch/s390/lib/usercopy.c | 8 --------
> arch/sparc/lib/Makefile | 1 -
> arch/sparc/lib/usercopy.c | 8 --------
> arch/tile/Kconfig | 8 +-------
> arch/tile/include/asm/uaccess.h | 7 ++++++-
> arch/tile/lib/uaccess.c | 8 --------
> arch/x86/Kconfig | 1 +
> arch/x86/Kconfig.debug | 14 --------------
> arch/x86/lib/usercopy_32.c | 6 ------
> arch/x86/lib/usercopy_64.c | 6 ------
> lib/Kconfig.debug | 18 ++++++++++++++++++
> lib/Makefile | 1 +
> lib/usercopy.c | 8 ++++++++
> 18 files changed, 37 insertions(+), 88 deletions(-)
> delete mode 100644 arch/s390/lib/usercopy.c
> delete mode 100644 arch/sparc/lib/usercopy.c
> create mode 100644 lib/usercopy.c

Very nice!

Acked-by: Ingo Molnar <[email protected]>

Ingo

2011-05-13 09:08:41

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH 1/9] iwlegacy: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

On Thu, May 12, 2011 at 04:50:04PM -0700, Stephen Boyd wrote:
> Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
> warning:
>
> In file included from arch/x86/include/asm/uaccess.h:573,
> from include/net/checksum.h:25,
> from include/linux/skbuff.h:28,
> from drivers/net/wireless/iwlegacy/iwl-4965-rs.c:28:
> In function 'copy_from_user',
> inlined from 'iwl4965_rs_sta_dbgfs_scale_table_write' at
> drivers/net/wireless/iwlegacy/iwl-4965-rs.c:2616:
> arch/x86/include/asm/uaccess_64.h:65:
> warning: call to 'copy_from_user_overflow' declared with
> attribute warning: copy_from_user() buffer size is not provably
> correct
>
> presumably due to buf_size being signed causing GCC to fail to
> see that buf_size can't become negative.
>
> Cc: Johannes Berg <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/net/wireless/iwlegacy/iwl-4965-rs.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> index 31ac672..cc4751c 100644
> --- a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> +++ b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> @@ -2604,7 +2604,7 @@ static ssize_t iwl4965_rs_sta_dbgfs_scale_table_write(struct file *file,
> struct iwl_lq_sta *lq_sta = file->private_data;
> struct iwl_priv *priv;
> char buf[64];
> - int buf_size;
> + size_t buf_size;

ACK, but we have more of that, perhaps you would like to fix them all.
If you don't want to, I'll do it. Note this can be easily done by:

sed -i 's/int buf_size;/size_t buf_size;/g' drivers/net/wireless/iwlegacy/*.[ch]

Stanislaw

2011-05-13 10:16:08

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 8/9] x86: Implement strict user copy checks for x86_64


* Stephen Boyd <[email protected]> wrote:

> Strict user copy checks are only really supported on x86_32 even
> though the config option is selectable on x86_64. Add the
> necessary support to the 64 bit code to trigger copy_from_user()
> warnings at compile time.
>
> Cc: [email protected]
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> arch/x86/include/asm/uaccess_64.h | 12 +++++++++---
> arch/x86/lib/usercopy_64.c | 6 ++++++
> 2 files changed, 15 insertions(+), 3 deletions(-)

Very nice!

Acked-by: Ingo Molnar <[email protected]>

Thanks,

Ingo

2011-05-14 20:34:11

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 9/9] Consolidate CONFIG_DEBUG_STRICT_USER_COPY_CHECKS

On Friday 13 May 2011, Stephen Boyd wrote:
> The help text for this config is duplicated across the x86,
> parisc, and s390 Kconfig.debug files. Arnd Bergman noted that the
> help text was slightly misleading and should be fixed to state
> that enabling this option isn't a problem when using pre 4.4 gcc.
>
> To simplify the rewording, consolidate the text into
> lib/Kconfig.debug and modify it there to be more explicit about
> when you should say N to this config.
>
> Also, make the text a bit more generic by stating that this
> option enables compile time checks so we can cover architectures
> which emit warnings vs. ones which emit errors. The details of
> how an architecture decided to implement the checks isn't as
> important as the concept of compile time checking of
> copy_from_user() calls.
>
> While we're doing this, remove all the copy_from_user_overflow()
> code that's duplicated many times and place it into lib/ so that
> any architecture supporting this option can get the function for
> free.
>
> Cc: Arnd Bergmann <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Arjan van de Ven <[email protected]>
> Cc: Helge Deller <[email protected]>
> Cc: Heiko Carstens <[email protected]>
> Cc: Stephen Rothwell <[email protected]>
> Cc: Chris Metcalf <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>

Thanks for following up on this

Reviewed-by: Arnd Bergmann <[email protected]>

2011-05-13 21:05:50

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 6/9] Bluetooth: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

Hi Stephen,

* Stephen Boyd <[email protected]> [2011-05-12 16:50:09 -0700]:

> Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
> warning:
>
> In function 'copy_from_user',
> inlined from 'rfcomm_sock_setsockopt' at
> net/bluetooth/rfcomm/sock.c:705:
> arch/x86/include/asm/uaccess_64.h:65:
> warning: call to 'copy_from_user_overflow' declared with
> attribute warning: copy_from_user() buffer size is not provably
> correct
>
> presumably due to buf_size being signed causing GCC to fail to
> see that buf_size can't become negative.
>
> Cc: Marcel Holtmann <[email protected]>
> Cc: Gustavo F. Padovan <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> net/bluetooth/rfcomm/sock.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)

Applied, thanks.

--
Gustavo F. Padovan
http://profusion.mobi

2011-05-13 23:25:30

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 4/9] debugfs: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

On Thu, May 12, 2011 at 04:50:07PM -0700, Stephen Boyd wrote:
> Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
> warning:
>
> In file included from arch/x86/include/asm/uaccess.h:573,
> from include/linux/uaccess.h:5,
> from include/linux/highmem.h:7,
> from include/linux/pagemap.h:10,
> from fs/debugfs/file.c:18:
> In function 'copy_from_user',
> inlined from 'write_file_bool' at fs/debugfs/file.c:435:
> arch/x86/include/asm/uaccess_64.h:65: warning: call to
> 'copy_from_user_overflow' declared with attribute warning:
> copy_from_user() buffer size is not provably correct
>
> presumably due to buf_size being signed causing GCC to fail to
> see that buf_size can't become negative.
>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> fs/debugfs/file.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index 89d394d..7ead5b8 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -428,7 +428,7 @@ static ssize_t write_file_bool(struct file *file, const char __user *user_buf,
> size_t count, loff_t *ppos)
> {
> char buf[32];
> - int buf_size;
> + size_t buf_size;
> u32 *val = file->private_data;

What tree did you make this against? It doesn't apply to the linux-next
tree :(

Anyway, I can hand edit it, not that big of a deal, but be more careful
the next time around.

greg k-h

2011-05-16 16:26:38

by Liam Girdwood

[permalink] [raw]
Subject: Re: [PATCH 7/9] ASoC: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

On Thu, 2011-05-12 at 16:50 -0700, Stephen Boyd wrote:
> Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
> warning:
>
> In file included from arch/x86/include/asm/uaccess.h:573,
> from include/linux/poll.h:14,
> from include/sound/pcm.h:29,
> from include/sound/ac97_codec.h:31,
> from sound/soc/soc-core.c:34:
> In function 'copy_from_user',
> inlined from 'codec_reg_write_file' at
> sound/soc/soc-core.c:252:
> arch/x86/include/asm/uaccess_64.h:65:
> warning: call to 'copy_from_user_overflow' declared with
> attribute warning: copy_from_user() buffer size is not provably
> correct
>
> presumably due to buf_size being signed causing GCC to fail to
> see that buf_size can't become negative.
>
> Cc: Liam Girdwood <[email protected]>
> Cc: Mark Brown <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>

Acked-by: Liam Girdwood <[email protected]>

2011-05-24 13:33:17

by James Smart

[permalink] [raw]
Subject: Re: [PATCH 3/9] [SCSI] lpfc: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning

Acked-by: James Smart <[email protected]>

Thanks

-- james


On 5/12/2011 7:50 PM, Stephen Boyd wrote:
> Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
> warning:
>
> In file included from arch/x86/include/asm/uaccess.h:573,
> from include/linux/uaccess.h:5,
> from include/linux/highmem.h:7,
> from include/linux/pagemap.h:10,
> from include/linux/blkdev.h:12,
> from drivers/scsi/lpfc/lpfc_debugfs.c:21:
> In function 'copy_from_user':
> arch/x86/include/asm/uaccess_64.h:65:
> warning: call to 'copy_from_user_overflow' declared with
> attribute warning: copy_from_user() buffer size is not provably
> correct
>
> presumably due to buf_size being signed causing GCC to fail to
> see that buf_size can't become negative.
>
> Cc: James Smart<[email protected]>
> Signed-off-by: Stephen Boyd<[email protected]>
> ---
> drivers/scsi/lpfc/lpfc_debugfs.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
> index 3d96774..0af53a6 100644
> --- a/drivers/scsi/lpfc/lpfc_debugfs.c
> +++ b/drivers/scsi/lpfc/lpfc_debugfs.c
> @@ -1305,7 +1305,8 @@ static int lpfc_idiag_cmd_get(const char __user *buf, size_t nbytes,
> {
> char mybuf[64];
> char *pbuf, *step_str;
> - int bsize, i;
> + int i;
> + size_t bsize;
>
> /* Protect copy from user */
> if (!access_ok(VERIFY_READ, buf, nbytes))

2011-05-24 21:29:23

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 0/9] strict user copy checks on x86_64

Hi Andrew,

(I don't know who to pick on sorry)

On 05/12/2011 04:50 PM, Stephen Boyd wrote:
> It turns out that strict user copy checks (also known as
> CONFIG_DEBUG_STRICT_USER_COPY_CHECKS) isn't actually implemented
> on x86_64 and thus we aren't catching potential security holes
> at compile time.
>
> This series adds support for strict user copy checks on x86_64
> and silences all the benign warnings in the x86_64 allyesconfig.
>
> The final patch consolidates the config option as its duplicated
> across mutliple arches. I don't know what tree this series should
> go through so I tried to send the individual driver patches to the
> respective maintainers.
>
> Stephen Boyd (9):
> iwlegacy: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
> iwlwifi: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
> [SCSI] lpfc: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
> debugfs: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
> kprobes: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
> Bluetooth: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
> ASoC: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
> x86: Implement strict user copy checks for x86_64
> Consolidate CONFIG_DEBUG_STRICT_USER_COPY_CHECKS

It looks like 1, 2, 4, 6, and 7 got picked up. Should I resend the left
over patches with appropriate acked-bys and tags? Would it be
appropriate to push this through your tree?

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-05-24 23:33:52

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 0/9] strict user copy checks on x86_64

On 05/24/2011 02:29 PM, Stephen Boyd wrote:
>
> It looks like 1, 2, 4, 6, and 7 got picked up. Should I resend the left
> over patches with appropriate acked-bys and tags? Would it be
> appropriate to push this through your tree?
>

I was first going to think I'd pick up 8 and 9 in tip, but since 9 is
cross-architecture, Andrew's tree might be better.

Acked-by: H. Peter Anvin <[email protected]>

-hpa