* [PATCH 0/4] RFC: tipc int vs size_t fixes
@ 2010-10-27 19:13 Paul Gortmaker
0 siblings, 0 replies; 8+ messages in thread
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 [flat|nested] 8+ messages in thread
* [PATCH 0/4] RFC: tipc int vs size_t fixes
@ 2010-10-27 19:29 Paul Gortmaker
2010-10-27 19:29 ` [PATCH 1/4] tipc: Fix bugs in tipc_msg_calc_data_size() Paul Gortmaker
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
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 [flat|nested] 8+ messages in thread
* [PATCH 1/4] tipc: Fix bugs in tipc_msg_calc_data_size()
2010-10-27 19:29 [PATCH 0/4] RFC: tipc int vs size_t fixes Paul Gortmaker
@ 2010-10-27 19:29 ` Paul Gortmaker
2010-10-28 14:07 ` Neil Horman
2010-10-27 19:29 ` [PATCH 2/4] tipc: Fix bugs in tipc_msg_build() Paul Gortmaker
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Paul Gortmaker @ 2010-10-27 19:29 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, torvalds, security
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 [flat|nested] 8+ messages in thread
* [PATCH 2/4] tipc: Fix bugs in tipc_msg_build()
2010-10-27 19:29 [PATCH 0/4] RFC: tipc int vs size_t fixes Paul Gortmaker
2010-10-27 19:29 ` [PATCH 1/4] tipc: Fix bugs in tipc_msg_calc_data_size() Paul Gortmaker
@ 2010-10-27 19:29 ` Paul Gortmaker
2010-10-28 14:19 ` Neil Horman
2010-10-27 19:29 ` [PATCH 3/4] tipc: Update arguments to use size_t for iovec array sizes Paul Gortmaker
2010-10-27 19:29 ` [PATCH 4/4] tipc: Fix bugs in sending of large amounts of byte-stream data Paul Gortmaker
3 siblings, 1 reply; 8+ messages in thread
From: Paul Gortmaker @ 2010-10-27 19:29 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, torvalds, security
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 [flat|nested] 8+ messages in thread
* [PATCH 3/4] tipc: Update arguments to use size_t for iovec array sizes
2010-10-27 19:29 [PATCH 0/4] RFC: tipc int vs size_t fixes Paul Gortmaker
2010-10-27 19:29 ` [PATCH 1/4] tipc: Fix bugs in tipc_msg_calc_data_size() Paul Gortmaker
2010-10-27 19:29 ` [PATCH 2/4] tipc: Fix bugs in tipc_msg_build() Paul Gortmaker
@ 2010-10-27 19:29 ` Paul Gortmaker
2010-10-27 19:29 ` [PATCH 4/4] tipc: Fix bugs in sending of large amounts of byte-stream data Paul Gortmaker
3 siblings, 0 replies; 8+ messages in thread
From: Paul Gortmaker @ 2010-10-27 19:29 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, torvalds, security
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 [flat|nested] 8+ messages in thread
* [PATCH 4/4] tipc: Fix bugs in sending of large amounts of byte-stream data
2010-10-27 19:29 [PATCH 0/4] RFC: tipc int vs size_t fixes Paul Gortmaker
` (2 preceding siblings ...)
2010-10-27 19:29 ` [PATCH 3/4] tipc: Update arguments to use size_t for iovec array sizes Paul Gortmaker
@ 2010-10-27 19:29 ` Paul Gortmaker
3 siblings, 0 replies; 8+ messages in thread
From: Paul Gortmaker @ 2010-10-27 19:29 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, drosenberg, jon.maloy, torvalds, security
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 [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] tipc: Fix bugs in tipc_msg_calc_data_size()
2010-10-27 19:29 ` [PATCH 1/4] tipc: Fix bugs in tipc_msg_calc_data_size() Paul Gortmaker
@ 2010-10-28 14:07 ` Neil Horman
0 siblings, 0 replies; 8+ messages in thread
From: Neil Horman @ 2010-10-28 14:07 UTC (permalink / raw)
To: Paul Gortmaker
Cc: davem, netdev, allan.stephens, drosenberg, jon.maloy, torvalds,
security
On Wed, Oct 27, 2010 at 03:29:30PM -0400, Paul Gortmaker wrote:
> 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
>
You should probably roll this patch together with your second one. The caller
tipc_msg_build will otherwise throw a warning when you build, as it assigns the
return value with this patch (a size_t) to a signed integer. It probably won't
matter since you limit dsz to TIPC_MAX_USER_MSG_SIZE which is small in
comparison to the size of an int, but nevertheless, the two patches are related,
so you can merge them.
Neil
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/4] tipc: Fix bugs in tipc_msg_build()
2010-10-27 19:29 ` [PATCH 2/4] tipc: Fix bugs in tipc_msg_build() Paul Gortmaker
@ 2010-10-28 14:19 ` Neil Horman
0 siblings, 0 replies; 8+ messages in thread
From: Neil Horman @ 2010-10-28 14:19 UTC (permalink / raw)
To: Paul Gortmaker
Cc: davem, netdev, allan.stephens, drosenberg, jon.maloy, torvalds,
security
On Wed, Oct 27, 2010 at 03:29:31PM -0400, Paul Gortmaker wrote:
> 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;
Don't you want to return -ETOOBIG here, or something simmilar? All the call
chains that I see for tipc_msg_build check for negative return codes and check
those against specific values. Why return some random too-big-value?
> }
>
> *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;
>
Ditto the above
> 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)
> {
I count 3 other callers of tipc_msg_calc_data_size (tipc_send,
tipc_forward2name, tipc_forward2port). Where are the updates to those functions
to make the corresponding size_t adjustments
Neil
> --
> 1.7.1.GIT
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-10-28 14:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-27 19:29 [PATCH 0/4] RFC: tipc int vs size_t fixes Paul Gortmaker
2010-10-27 19:29 ` [PATCH 1/4] tipc: Fix bugs in tipc_msg_calc_data_size() Paul Gortmaker
2010-10-28 14:07 ` Neil Horman
2010-10-27 19:29 ` [PATCH 2/4] tipc: Fix bugs in tipc_msg_build() Paul Gortmaker
2010-10-28 14:19 ` Neil Horman
2010-10-27 19:29 ` [PATCH 3/4] tipc: Update arguments to use size_t for iovec array sizes Paul Gortmaker
2010-10-27 19:29 ` [PATCH 4/4] tipc: Fix bugs in sending of large amounts of byte-stream data Paul Gortmaker
-- strict thread matches above, loose matches on Subject: below --
2010-10-27 19:13 [PATCH 0/4] RFC: tipc int vs size_t fixes Paul Gortmaker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).