* Re: [PATCH] tunnels: Fix tunnels change rcu protection
From: Eric Dumazet @ 2010-10-27 19:06 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, Linux Netdev List
In-Reply-To: <1288206172.2658.11.camel@edumazet-laptop>
Le mercredi 27 octobre 2010 à 21:02 +0200, Eric Dumazet a écrit :
> Hmm, maybe we should allocate a "struct ip_tunnel_parm" instead of using
> an embedded one (in struct ip_tunnel), and stick an rcu_head in it to
> delay its freeing...
>
I forgot to Ack your patch, of course.
We can implement something better when net-next-2.6 re-opens.
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [Security] TIPC security issues
From: Paul Gortmaker @ 2010-10-27 19:00 UTC (permalink / raw)
To: David Miller
Cc: torvalds, drosenberg, jon.maloy, allan.stephens, netdev, security
In-Reply-To: <20101027.113537.15227235.davem@davemloft.net>
On 10-10-27 02:35 PM, David Miller wrote:
> From: Paul Gortmaker<paul.gortmaker@windriver.com>
> Date: Wed, 27 Oct 2010 14:27:13 -0400
>
>> I've got some patches from Al that I'm pre-reviewing/testing
>> before spamming everyone with them (will send within 24h, if
>> that is OK). Anyway, in the above, does it make sense to
>> check for the overflow incrementally, i.e. something like this?
>
> Can you please not work on this in private? That's the only reason
> we have duplicated work here.
Sorry -- my intent was to not waste people's time with stuff
that might have obvious issues. I'll send what I have now
and we can go from there.
P.
^ permalink raw reply
* [PATCH 0/4] RFC: tipc int vs size_t fixes
From: Paul Gortmaker @ 2010-10-27 19:13 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, security
This is a starting point for fixing up the mix of int vs. size_t
usage that Dan spotted in TIPC as being open to possible exploits.
Open questions I had remaining with these patches were whether we could
trim out some of the unrequired casts, and whether we wanted to use a
size_t for everything that at any time was based on or compared to
an iov_len, including all instances of num_sect, when things like
iov_length() in <linux/uio.h> use unsigned long for looping over segments.
Also, whether we should give up at INT_MAX or LONG_MAX...
Please suggest changes as required, and I'll integrate them into this
pass1 of draft of patches from Al and resend as required until we've
got a final acceptable solution.
Thanks,
Paul.
^ permalink raw reply
* Re: [PATCH 1/4] tipc: Fix bugs in tipc_msg_calc_data_size()
From: David Miller @ 2010-10-27 19:17 UTC (permalink / raw)
To: paul.gortmaker; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, security
In-Reply-To: <1288206816-23025-2-git-send-email-paul.gortmaker@windriver.com>
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Wed, 27 Oct 2010 15:13:33 -0400
> From: Allan Stephens <Allan.Stephens@windriver.com>
>
> Enhances TIPC's computation of the amount of data to be sent so that
> it works properly when large values are involved. Calculations are now
> done using "size_t" instead of "int", and a check has been added to
> handle cases where the total amount of data exceeds the range of "size_t".
>
> Signed-off-by: Allan Stephens <Allan.Stephens@windriver.com>
The protocol socket I/O call ops can't even return anything larger
than an 'int' because of the signature for those function pointers
(check out *sendmsg and *recvmsg in include/linux/net.h).
So returning "long" from here doesn't make any sense.
You really have to limit the usable lengths to the range of
an 'int' all the way up to the code in net/socket.c
^ permalink raw reply
* [PATCH] pktgen: Remove a dangerous debug print.
From: Nelson Elhage @ 2010-10-27 19:13 UTC (permalink / raw)
To: Robert Olsson; +Cc: linux-kernel, netdev, Eugene Teo, Nelson Elhage
We were allocating an arbitrarily-large buffer on the stack, which would allow a
buggy or malicious userspace program to overflow the kernel stack.
Since the debug printk() was just printing exactly the text passed from
userspace, it's probably just as easy for anyone who might use it to augment (or
just strace(1)) the program writing to the pktgen file, so let's just not bother
trying to print the whole buffer.
Signed-off-by: Nelson Elhage <nelhage@ksplice.com>
---
net/core/pktgen.c | 11 +++--------
1 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 10a1ea7..de8e0da 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -888,14 +888,9 @@ static ssize_t pktgen_if_write(struct file *file,
i += len;
- if (debug) {
- char tb[count + 1];
- if (copy_from_user(tb, user_buffer, count))
- return -EFAULT;
- tb[count] = 0;
- printk(KERN_DEBUG "pktgen: %s,%lu buffer -:%s:-\n", name,
- (unsigned long)count, tb);
- }
+ if (debug)
+ printk(KERN_DEBUG "pktgen: %s,%lu\n", name,
+ (unsigned long)count);
if (!strcmp(name, "min_pkt_size")) {
len = num_arg(&user_buffer[i], 10, &value);
--
1.7.1.31.g6297e
^ permalink raw reply related
* Re: [PATCH] pktgen: Remove a dangerous debug print.
From: David Miller @ 2010-10-27 19:21 UTC (permalink / raw)
To: nelhage; +Cc: robert.olsson, linux-kernel, netdev, eugene
In-Reply-To: <1288206788-21063-1-git-send-email-nelhage@ksplice.com>
From: Nelson Elhage <nelhage@ksplice.com>
Date: Wed, 27 Oct 2010 15:13:08 -0400
> We were allocating an arbitrarily-large buffer on the stack, which would allow a
> buggy or malicious userspace program to overflow the kernel stack.
>
> Since the debug printk() was just printing exactly the text passed from
> userspace, it's probably just as easy for anyone who might use it to augment (or
> just strace(1)) the program writing to the pktgen file, so let's just not bother
> trying to print the whole buffer.
>
> Signed-off-by: Nelson Elhage <nelhage@ksplice.com>
Only root can write to the pktgen control file.
Also, the debug feature really is used by people's pktgen scripts, you
can't just turn it off.
^ permalink raw reply
* Re: [PATCH 5/14] drivers/net/sb1000.c: delete double assignment
From: David Miller @ 2010-10-27 19:23 UTC (permalink / raw)
To: julia; +Cc: netdev, kernel-janitors, linux-kernel
In-Reply-To: <1288088743-3725-6-git-send-email-julia@diku.dk>
From: Julia Lawall <julia@diku.dk>
Date: Tue, 26 Oct 2010 12:25:34 +0200
> From: Julia Lawall <julia@diku.dk>
>
> The other code around these duplicated assignments initializes the 0 1 2
> and 3 elements of an array, so change the initialization of the
> rx_session_id array to do the same.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <julia@diku.dk>
Applied.
^ permalink raw reply
* Re: [PATCH 7/14] drivers/net/typhoon.c: delete double assignment
From: David Miller @ 2010-10-27 19:24 UTC (permalink / raw)
To: julia; +Cc: dave, kernel-janitors, netdev, linux-kernel
In-Reply-To: <1288088743-3725-8-git-send-email-julia@diku.dk>
From: Julia Lawall <julia@diku.dk>
Date: Tue, 26 Oct 2010 12:25:36 +0200
> From: Julia Lawall <julia@diku.dk>
>
> Delete successive assignments to the same location. The current definition
> does not initialize the respRing structure, which has the same type as the
> cmdRing structure, so initialize that one instead.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <julia@diku.dk>
Applied.
^ permalink raw reply
* Re: [PATCHv2 0/3]qlcnic: bug fixes
From: David Miller @ 2010-10-27 19:24 UTC (permalink / raw)
To: amit.salecha; +Cc: netdev, ameen.rahman, anirban.chakraborty
In-Reply-To: <1288151589-32431-1-git-send-email-amit.salecha@qlogic.com>
From: Amit Kumar Salecha <amit.salecha@qlogic.com>
Date: Tue, 26 Oct 2010 20:53:06 -0700
> Series v2 of 3 patches for bug fixes. Patches are numbered.
> Dropping "dma address align check" patch as pci_alloc_consistent is gaurantee to give
> page align dma address.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 10/14] drivers/isdn: delete double assignment
From: David Miller @ 2010-10-27 19:24 UTC (permalink / raw)
To: julia; +Cc: wharms, isdn, kernel-janitors, netdev, linux-kernel
In-Reply-To: <Pine.LNX.4.64.1010261419510.25281@ask.diku.dk>
From: Julia Lawall <julia@diku.dk>
Date: Tue, 26 Oct 2010 14:20:56 +0200 (CEST)
> From: Julia Lawall <julia@diku.dk>
>
> Delete successive assignments to the same location. In the first case, the
> hscx array has two elements, so change the assignment to initialize the
> second one. In the second case, the two assignments are simply identical.
> Furthermore, neither is necessary, because the effect of the assignment is
> only visible in the next line, in the assignment in the if test. The patch
> inlines the right hand side value in the latter assignment and pulls that
> assignment out of the if test.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <julia@diku.dk>
Applied.
^ permalink raw reply
* Re: [Security] TIPC security issues
From: David Miller @ 2010-10-27 19:27 UTC (permalink / raw)
To: torvalds; +Cc: drosenberg, jon.maloy, allan.stephens, netdev, security
In-Reply-To: <AANLkTikKZdq9u0+Nkd4wtEcAZMT3YCPaXMv6FZaafetc@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 27 Oct 2010 11:51:19 -0700
> So doing this in verify_iovec() (and verify_compat_iovec - which I
> didn't do in my RFC patch) really does fix everything, and means that
> the individual socket types never have to worry about the subtle cases
> of overflow in any type.
I completely agree about capping things in verify_iovec().
And it was completely erroneous of me to change verify_iovec() to
return 'long' instead of 'int', it should have stayed at 'int' with a
cap.
Because the protocols don't even have a way to return something larger
than an 'int' as a return value due to the signature of the recvmsg()
and sendmsg() socket ops.
^ permalink raw reply
* Re: [PATCH] pktgen: Remove a dangerous debug print.
From: Nelson Elhage @ 2010-10-27 19:28 UTC (permalink / raw)
To: David Miller; +Cc: robert.olsson, linux-kernel, netdev, eugene
In-Reply-To: <20101027.122143.02260950.davem@davemloft.net>
How would you feel about limiting the debug print to at most, say, 512 or 1024
bytes? Even if it's only accessible to root by default, I don't a userspace
program should be able to accidentally corrupt the kernel stack by writing too
many bytes to a file in /proc.
- Nelson
On Wed, Oct 27, 2010 at 12:21:43PM -0700, David Miller wrote:
> From: Nelson Elhage <nelhage@ksplice.com>
> Date: Wed, 27 Oct 2010 15:13:08 -0400
>
> > We were allocating an arbitrarily-large buffer on the stack, which would allow a
> > buggy or malicious userspace program to overflow the kernel stack.
> >
> > Since the debug printk() was just printing exactly the text passed from
> > userspace, it's probably just as easy for anyone who might use it to augment (or
> > just strace(1)) the program writing to the pktgen file, so let's just not bother
> > trying to print the whole buffer.
> >
> > Signed-off-by: Nelson Elhage <nelhage@ksplice.com>
>
> Only root can write to the pktgen control file.
>
> Also, the debug feature really is used by people's pktgen scripts, you
> can't just turn it off.
^ permalink raw reply
* Re: [PATCH] pktgen: Remove a dangerous debug print.
From: David Miller @ 2010-10-27 19:30 UTC (permalink / raw)
To: nelhage; +Cc: robert.olsson, linux-kernel, netdev, eugene
In-Reply-To: <20101027192808.GP16803@ksplice.com>
From: Nelson Elhage <nelhage@ksplice.com>
Date: Wed, 27 Oct 2010 15:28:08 -0400
> How would you feel about limiting the debug print to at most, say, 512 or 1024
> bytes? Even if it's only accessible to root by default, I don't a userspace
> program should be able to accidentally corrupt the kernel stack by writing too
> many bytes to a file in /proc.
Why not? He can just as easily "cat whatever >/dev/kmem" or similar?
And I'm sure there are other proc files that can cause similar damage
such as the PCI device control files.
^ permalink raw reply
* Re: [PATCHv2 1/4] caif-u5500: Adding shared memory include
From: David Miller @ 2010-10-27 19:31 UTC (permalink / raw)
To: sjur.brandeland
Cc: netdev, linux, linux-arm-kernel, linus.walleij, stefan.xk.nilsson,
amarnath.bangalore.revanna
In-Reply-To: <1288204482-2742-1-git-send-email-sjur.brandeland@stericsson.com>
From: Sjur Braendeland <sjur.brandeland@stericsson.com>
Date: Wed, 27 Oct 2010 20:34:39 +0200
> From: Amarnath Revanna <amarnath.bangalore.revanna@stericsson.com>
>
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCHv2 2/4] caif-u5500: CAIF shared memory transport protocol
From: David Miller @ 2010-10-27 19:31 UTC (permalink / raw)
To: sjur.brandeland
Cc: netdev, linux, linux-arm-kernel, linus.walleij, stefan.xk.nilsson
In-Reply-To: <1288204482-2742-2-git-send-email-sjur.brandeland@stericsson.com>
From: Sjur Braendeland <sjur.brandeland@stericsson.com>
Date: Wed, 27 Oct 2010 20:34:40 +0200
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCHv2 3/4] caif-u5500: CAIF shared memory mailbox interface
From: David Miller @ 2010-10-27 19:31 UTC (permalink / raw)
To: sjur.brandeland
Cc: netdev, linux, linux-arm-kernel, linus.walleij, stefan.xk.nilsson,
amarnath.bangalore.revanna
In-Reply-To: <1288204482-2742-3-git-send-email-sjur.brandeland@stericsson.com>
From: Sjur Braendeland <sjur.brandeland@stericsson.com>
Date: Wed, 27 Oct 2010 20:34:41 +0200
> From: Amarnath Revanna <amarnath.bangalore.revanna@stericsson.com>
>
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCHv2 4/4] caif-u5500: Build config for CAIF shared mem driver
From: David Miller @ 2010-10-27 19:31 UTC (permalink / raw)
To: sjur.brandeland
Cc: netdev, linux, linux-arm-kernel, linus.walleij, stefan.xk.nilsson,
amarnath.bangalore.revanna
In-Reply-To: <1288204482-2742-4-git-send-email-sjur.brandeland@stericsson.com>
From: Sjur Braendeland <sjur.brandeland@stericsson.com>
Date: Wed, 27 Oct 2010 20:34:42 +0200
> From: Amarnath Revanna <amarnath.bangalore.revanna@stericsson.com>
>
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* [PATCH 2/4] tipc: Fix bugs in tipc_msg_build()
From: Paul Gortmaker @ 2010-10-27 19:29 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, torvalds, security
In-Reply-To: <1288207773-25448-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <Allan.Stephens@windriver.com>
Enhances TIPC's creation of message buffers so that it works properly
when large amounts of data are involved. Calculations are now done
using "size_t" where needed, and comparisons no longer mix signed and
unsigned size values.
Signed-off-by: Allan Stephens <Allan.Stephens@windriver.com>
---
net/tipc/msg.c | 25 +++++++++++++++++--------
net/tipc/msg.h | 4 ++--
2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index 38360a9..b67e831 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -96,27 +96,36 @@ size_t tipc_msg_calc_data_size(struct iovec const *msg_sect, size_t num_sect)
*
* Note: Caller must not hold any locks in case copy_from_user() is interrupted!
*
- * Returns message data size or errno
+ * If successful, creates message buffer and returns message data size
+ * (which must be <= TIPC_MAX_USER_MSG_SIZE).
+ * If fails only because message data size exceeds the specified maximum
+ * returns message data size, but doesn't created a message buffer.
+ * If fails for any other reason returns errno and doesn't create a buffer.
*/
int tipc_msg_build(struct tipc_msg *hdr,
- struct iovec const *msg_sect, u32 num_sect,
- int max_size, int usrmem, struct sk_buff** buf)
+ struct iovec const *msg_sect, size_t num_sect,
+ u32 max_size, int usrmem, struct sk_buff **buf)
{
- int dsz, sz, hsz, pos, res, cnt;
+ size_t dsz;
+ u32 hsz;
+ u32 sz;
+ size_t pos;
+ size_t cnt;
+ int res;
dsz = tipc_msg_calc_data_size(msg_sect, num_sect);
- if (unlikely(dsz > TIPC_MAX_USER_MSG_SIZE)) {
+ if (unlikely(dsz > (size_t)TIPC_MAX_USER_MSG_SIZE)) {
*buf = NULL;
return -EINVAL;
}
pos = hsz = msg_hdr_sz(hdr);
- sz = hsz + dsz;
+ sz = hsz + (u32)dsz;
msg_set_size(hdr, sz);
if (unlikely(sz > max_size)) {
*buf = NULL;
- return dsz;
+ return (int)dsz;
}
*buf = tipc_buf_acquire(sz);
@@ -135,7 +144,7 @@ int tipc_msg_build(struct tipc_msg *hdr,
pos += msg_sect[cnt].iov_len;
}
if (likely(res))
- return dsz;
+ return (int)dsz;
buf_discard(*buf);
*buf = NULL;
diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index a132800..41fb532 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -713,8 +713,8 @@ void tipc_msg_init(struct tipc_msg *m, u32 user, u32 type,
u32 hsize, u32 destnode);
size_t tipc_msg_calc_data_size(struct iovec const *msg_sect, size_t num_sect);
int tipc_msg_build(struct tipc_msg *hdr,
- struct iovec const *msg_sect, u32 num_sect,
- int max_size, int usrmem, struct sk_buff** buf);
+ struct iovec const *msg_sect, size_t num_sect,
+ u32 max_size, int usrmem, struct sk_buff **buf);
static inline void msg_set_media_addr(struct tipc_msg *m, struct tipc_media_addr *a)
{
--
1.7.1.GIT
^ permalink raw reply related
* [PATCH 1/4] tipc: Fix bugs in tipc_msg_calc_data_size()
From: Paul Gortmaker @ 2010-10-27 19:29 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, torvalds, security
In-Reply-To: <1288207773-25448-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <Allan.Stephens@windriver.com>
Enhances TIPC's computation of the amount of data to be sent so that
it works properly when large values are involved. Calculations are now
done using "size_t" instead of "int", and a check has been added to
handle cases where the total amount of data exceeds the range of "size_t".
Signed-off-by: Allan Stephens <Allan.Stephens@windriver.com>
---
net/tipc/msg.c | 17 ++++++++++++-----
net/tipc/msg.h | 2 +-
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index ecb532f..38360a9 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -72,15 +72,22 @@ void tipc_msg_init(struct tipc_msg *m, u32 user, u32 type,
/**
* tipc_msg_calc_data_size - determine total data size for message
+ *
+ * Note: If total exceeds range of size_t returns largest possible value
*/
-int tipc_msg_calc_data_size(struct iovec const *msg_sect, u32 num_sect)
+size_t tipc_msg_calc_data_size(struct iovec const *msg_sect, size_t num_sect)
{
- int dsz = 0;
- int i;
+ size_t dsz = 0;
+ size_t len;
+ size_t i;
- for (i = 0; i < num_sect; i++)
- dsz += msg_sect[i].iov_len;
+ for (i = 0; i < num_sect; i++) {
+ len = msg_sect[i].iov_len;
+ if (len > (size_t)LONG_MAX - dsz)
+ return (size_t)LONG_MAX;
+ dsz += len;
+ }
return dsz;
}
diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index 031aad1..a132800 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -711,7 +711,7 @@ static inline void msg_set_dataoctet(struct tipc_msg *m, u32 pos)
u32 tipc_msg_tot_importance(struct tipc_msg *m);
void tipc_msg_init(struct tipc_msg *m, u32 user, u32 type,
u32 hsize, u32 destnode);
-int tipc_msg_calc_data_size(struct iovec const *msg_sect, u32 num_sect);
+size_t tipc_msg_calc_data_size(struct iovec const *msg_sect, size_t num_sect);
int tipc_msg_build(struct tipc_msg *hdr,
struct iovec const *msg_sect, u32 num_sect,
int max_size, int usrmem, struct sk_buff** buf);
--
1.7.1.GIT
^ permalink raw reply related
* [PATCH 4/4] tipc: Fix bugs in sending of large amounts of byte-stream data
From: Paul Gortmaker @ 2010-10-27 19:29 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, torvalds, security
In-Reply-To: <1288207773-25448-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <Allan.Stephens@windriver.com>
Enhances the routine that sends data for SOCK_STREAM sockets to
fix the following issues:
- Uses "size_t" to specify the size and number of iovec array entries
to avoid the risk of accidentally truncating data.
- No longer uses comparisons that mix signed and unsigned size values.
- Adds check to terminate sending early if continuation would require
returning a value too large to fit in an "int".
Signed-off-by: Allan Stephens <Allan.Stephens@windriver.com>
---
net/tipc/socket.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 33217fc..081eda5 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -703,12 +703,12 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
struct msghdr my_msg;
struct iovec my_iov;
struct iovec *curr_iov;
- int curr_iovlen;
+ size_t curr_iovlen;
char __user *curr_start;
u32 hdr_size;
- int curr_left;
- int bytes_to_send;
- int bytes_sent;
+ size_t curr_left;
+ size_t bytes_to_send;
+ size_t bytes_sent;
int res;
lock_sock(sk);
@@ -757,15 +757,19 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
while (curr_left) {
bytes_to_send = tport->max_pkt - hdr_size;
- if (bytes_to_send > TIPC_MAX_USER_MSG_SIZE)
- bytes_to_send = TIPC_MAX_USER_MSG_SIZE;
+ if (bytes_to_send > (size_t)TIPC_MAX_USER_MSG_SIZE)
+ bytes_to_send = (size_t)TIPC_MAX_USER_MSG_SIZE;
if (curr_left < bytes_to_send)
bytes_to_send = curr_left;
+ if (bytes_to_send > (size_t)INT_MAX - bytes_sent) {
+ res = (int)bytes_sent;
+ goto exit;
+ }
my_iov.iov_base = curr_start;
my_iov.iov_len = bytes_to_send;
if ((res = send_packet(NULL, sock, &my_msg, 0)) < 0) {
if (bytes_sent)
- res = bytes_sent;
+ res = (int)bytes_sent;
goto exit;
}
curr_left -= bytes_to_send;
@@ -775,7 +779,7 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
curr_iov++;
}
- res = bytes_sent;
+ res = (int)bytes_sent;
exit:
release_sock(sk);
return res;
--
1.7.1.GIT
^ permalink raw reply related
* [PATCH 3/4] tipc: Update arguments to use size_t for iovec array sizes
From: Paul Gortmaker @ 2010-10-27 19:29 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, torvalds, security
In-Reply-To: <1288207773-25448-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <Allan.Stephens@windriver.com>
Ensures that all routines that pass iovec arrays as arguments specify
the array length using "size_t" to avoid the risk of accidentally
truncating a large array.
Signed-off-by: Allan Stephens <Allan.Stephens@windriver.com>
---
include/net/tipc/tipc.h | 8 ++++----
net/tipc/link.c | 6 +++---
net/tipc/link.h | 2 +-
net/tipc/port.c | 16 ++++++++--------
net/tipc/port.h | 2 +-
5 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/include/net/tipc/tipc.h b/include/net/tipc/tipc.h
index 1e0645e..7750e2b 100644
--- a/include/net/tipc/tipc.h
+++ b/include/net/tipc/tipc.h
@@ -157,18 +157,18 @@ int tipc_shutdown(u32 ref);
int tipc_send(u32 portref,
- unsigned int num_sect,
+ size_t num_sect,
struct iovec const *msg_sect);
int tipc_send2name(u32 portref,
struct tipc_name const *name,
u32 domain,
- unsigned int num_sect,
+ size_t num_sect,
struct iovec const *msg_sect);
int tipc_send2port(u32 portref,
struct tipc_portid const *dest,
- unsigned int num_sect,
+ size_t num_sect,
struct iovec const *msg_sect);
int tipc_send_buf2port(u32 portref,
@@ -179,7 +179,7 @@ int tipc_send_buf2port(u32 portref,
int tipc_multicast(u32 portref,
struct tipc_name_seq const *seq,
u32 domain, /* currently unused */
- unsigned int section_count,
+ size_t section_count,
struct iovec const *msg);
#endif
diff --git a/net/tipc/link.c b/net/tipc/link.c
index a997d9f..a92099f 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -106,7 +106,7 @@ static int link_recv_changeover_msg(struct link **l_ptr, struct sk_buff **buf);
static void link_set_supervision_props(struct link *l_ptr, u32 tolerance);
static int link_send_sections_long(struct port *sender,
struct iovec const *msg_sect,
- u32 num_sect, u32 destnode);
+ size_t num_sect, u32 destnode);
static void link_check_defragm_bufs(struct link *l_ptr);
static void link_state_event(struct link *l_ptr, u32 event);
static void link_reset_statistics(struct link *l_ptr);
@@ -1180,7 +1180,7 @@ int tipc_send_buf_fast(struct sk_buff *buf, u32 destnode)
*/
int tipc_link_send_sections_fast(struct port *sender,
struct iovec const *msg_sect,
- const u32 num_sect,
+ const size_t num_sect,
u32 destaddr)
{
struct tipc_msg *hdr = &sender->publ.phdr;
@@ -1276,7 +1276,7 @@ exit:
*/
static int link_send_sections_long(struct port *sender,
struct iovec const *msg_sect,
- u32 num_sect,
+ size_t num_sect,
u32 destaddr)
{
struct link *l_ptr;
diff --git a/net/tipc/link.h b/net/tipc/link.h
index f98bc61..8832c78 100644
--- a/net/tipc/link.h
+++ b/net/tipc/link.h
@@ -236,7 +236,7 @@ int tipc_link_send_buf(struct link *l_ptr, struct sk_buff *buf);
u32 tipc_link_get_max_pkt(u32 dest,u32 selector);
int tipc_link_send_sections_fast(struct port* sender,
struct iovec const *msg_sect,
- const u32 num_sect,
+ const size_t num_sect,
u32 destnode);
void tipc_link_recv_bundle(struct sk_buff *buf);
int tipc_link_recv_fragment(struct sk_buff **pending,
diff --git a/net/tipc/port.c b/net/tipc/port.c
index 82092ea..c65409b 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -95,7 +95,7 @@ static void port_incr_out_seqno(struct port *p_ptr)
*/
int tipc_multicast(u32 ref, struct tipc_name_seq const *seq, u32 domain,
- u32 num_sect, struct iovec const *msg_sect)
+ size_t num_sect, struct iovec const *msg_sect)
{
struct tipc_msg *hdr;
struct sk_buff *buf;
@@ -446,7 +446,7 @@ int tipc_reject_msg(struct sk_buff *buf, u32 err)
}
int tipc_port_reject_sections(struct port *p_ptr, struct tipc_msg *hdr,
- struct iovec const *msg_sect, u32 num_sect,
+ struct iovec const *msg_sect, size_t num_sect,
int err)
{
struct sk_buff *buf;
@@ -1219,7 +1219,7 @@ int tipc_shutdown(u32 ref)
* message for this node.
*/
-static int tipc_port_recv_sections(struct port *sender, unsigned int num_sect,
+static int tipc_port_recv_sections(struct port *sender, size_t num_sect,
struct iovec const *msg_sect)
{
struct sk_buff *buf;
@@ -1236,7 +1236,7 @@ static int tipc_port_recv_sections(struct port *sender, unsigned int num_sect,
* tipc_send - send message sections on connection
*/
-int tipc_send(u32 ref, unsigned int num_sect, struct iovec const *msg_sect)
+int tipc_send(u32 ref, size_t num_sect, struct iovec const *msg_sect)
{
struct port *p_ptr;
u32 destnode;
@@ -1277,7 +1277,7 @@ int tipc_send(u32 ref, unsigned int num_sect, struct iovec const *msg_sect)
static int tipc_forward2name(u32 ref,
struct tipc_name const *name,
u32 domain,
- u32 num_sect,
+ size_t num_sect,
struct iovec const *msg_sect,
struct tipc_portid const *orig,
unsigned int importance)
@@ -1331,7 +1331,7 @@ static int tipc_forward2name(u32 ref,
int tipc_send2name(u32 ref,
struct tipc_name const *name,
unsigned int domain,
- unsigned int num_sect,
+ size_t num_sect,
struct iovec const *msg_sect)
{
struct tipc_portid orig;
@@ -1348,7 +1348,7 @@ int tipc_send2name(u32 ref,
static int tipc_forward2port(u32 ref,
struct tipc_portid const *dest,
- unsigned int num_sect,
+ size_t num_sect,
struct iovec const *msg_sect,
struct tipc_portid const *orig,
unsigned int importance)
@@ -1389,7 +1389,7 @@ static int tipc_forward2port(u32 ref,
int tipc_send2port(u32 ref,
struct tipc_portid const *dest,
- unsigned int num_sect,
+ size_t num_sect,
struct iovec const *msg_sect)
{
struct tipc_portid orig;
diff --git a/net/tipc/port.h b/net/tipc/port.h
index 73bbf44..baa2e71 100644
--- a/net/tipc/port.h
+++ b/net/tipc/port.h
@@ -110,7 +110,7 @@ extern spinlock_t tipc_port_list_lock;
struct port_list;
int tipc_port_reject_sections(struct port *p_ptr, struct tipc_msg *hdr,
- struct iovec const *msg_sect, u32 num_sect,
+ struct iovec const *msg_sect, size_t num_sect,
int err);
struct sk_buff *tipc_port_get_ports(void);
struct sk_buff *port_show_stats(const void *req_tlv_area, int req_tlv_space);
--
1.7.1.GIT
^ permalink raw reply related
* [PATCH 0/4] RFC: tipc int vs size_t fixes
From: Paul Gortmaker @ 2010-10-27 19:29 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, torvalds, security
[apologies if you get this 2x ; a bogus Status: header caused them
to fail vger's sanity check for netdev, so resending.]
This is a starting point for fixing up the mix of int vs. size_t
usage that Dan spotted in TIPC as being open to possible exploits.
Open questions I had remaining with these patches were whether we could
trim out some of the unrequired casts, and whether we wanted to use a
size_t for everything that at any time was based on or compared to
an iov_len, including all instances of num_sect, when things like
iov_length() in <linux/uio.h> use unsigned long for looping over segments.
Also, whether we should give up at INT_MAX or LONG_MAX...
Please suggest changes as required, and I'll integrate them into this
pass1 of draft of patches from Al and resend as required until we've
got a final acceptable solution.
Thanks,
Paul.
^ permalink raw reply
* Re: [PATCH] pktgen: Remove a dangerous debug print.
From: Eric Dumazet @ 2010-10-27 19:41 UTC (permalink / raw)
To: Nelson Elhage; +Cc: David Miller, robert.olsson, linux-kernel, netdev, eugene
In-Reply-To: <20101027192808.GP16803@ksplice.com>
Le mercredi 27 octobre 2010 à 15:28 -0400, Nelson Elhage a écrit :
> How would you feel about limiting the debug print to at most, say, 512 or 1024
> bytes? Even if it's only accessible to root by default, I don't a userspace
> program should be able to accidentally corrupt the kernel stack by writing too
> many bytes to a file in /proc.
Arent /proc writes limited to PAGE_SIZE anyway ?
On x86 at least, you cannot corrupt kernel stack, since its bigger than
PAGE_SIZE.
I agree pktgen code is a bit ugly and needs a cleanup, but who
cares ? :)
^ permalink raw reply
* Re: [PATCH] pktgen: Remove a dangerous debug print.
From: Nelson Elhage @ 2010-10-27 19:49 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, robert.olsson, linux-kernel, netdev, eugene
In-Reply-To: <1288208499.2658.19.camel@edumazet-laptop>
I tested this and was able to oops both amd64 and i386 test machines with 8k
writes to the pktgen file. I haven't investigated whether that's because there's
no PAGE_SIZE limit, or because one page ends up being enough to cause a problem
on all my test machines.
- Nelson
On Wed, Oct 27, 2010 at 09:41:39PM +0200, Eric Dumazet wrote:
> Le mercredi 27 octobre 2010 à 15:28 -0400, Nelson Elhage a écrit :
> > How would you feel about limiting the debug print to at most, say, 512 or 1024
> > bytes? Even if it's only accessible to root by default, I don't a userspace
> > program should be able to accidentally corrupt the kernel stack by writing too
> > many bytes to a file in /proc.
>
> Arent /proc writes limited to PAGE_SIZE anyway ?
>
> On x86 at least, you cannot corrupt kernel stack, since its bigger than
> PAGE_SIZE.
>
> I agree pktgen code is a bit ugly and needs a cleanup, but who
> cares ? :)
>
>
>
>
^ permalink raw reply
* Re: [PATCH] pktgen: Remove a dangerous debug print.
From: Ben Greear @ 2010-10-27 20:38 UTC (permalink / raw)
To: Nelson Elhage; +Cc: Robert Olsson, linux-kernel, netdev, Eugene Teo
In-Reply-To: <1288206788-21063-1-git-send-email-nelhage@ksplice.com>
On 10/27/2010 12:13 PM, Nelson Elhage wrote:
> We were allocating an arbitrarily-large buffer on the stack, which would allow a
> buggy or malicious userspace program to overflow the kernel stack.
>
> Since the debug printk() was just printing exactly the text passed from
> userspace, it's probably just as easy for anyone who might use it to augment (or
> just strace(1)) the program writing to the pktgen file, so let's just not bother
> trying to print the whole buffer.
Maybe just allocate that buffer on the heap instead of stack?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox