* [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
* 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
* [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
* 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
* [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
* [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
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).