Messages in this thread Patch in this message | | | Date | Sat, 2 Nov 2013 17:35:12 -0700 | Subject | Re: [PATCH 1/2] ipc, msg: fix message length check for negative values | From | Linus Torvalds <> |
| |
On Sat, Nov 2, 2013 at 2:26 PM, Mathias Krause <minipli@googlemail.com> wrote: > On 64 bit systems the test for negative message sizes is bogus as the > size, which may be positive when evaluated as a long, will get truncated > to an int when passed to load_msg().
Quite frankly, wouldn't it be much nicer to just fix "load_msg()" instead?
Using "size_t" also gets rid of the games we play with DATALEN_MSG/SEG.
IOW, something like the attached..
Of course, we *also* should fix ns->msg_ctlmax to make clear you can't use negative numbers there. No question about that. I think it would be better to even avoid INT_MAX, because there are memory use concerns and CPU usage ones too (we generate that list of smaller-than-page-size fragments).
Hmm?
Linus include/linux/ipc_namespace.h | 6 +++--- ipc/msgutil.c | 14 +++++++------- ipc/util.h | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h index 19c19a5eee29..f6c82de12541 100644 --- a/include/linux/ipc_namespace.h +++ b/include/linux/ipc_namespace.h @@ -34,9 +34,9 @@ struct ipc_namespace { int sem_ctls[4]; int used_sems; - int msg_ctlmax; - int msg_ctlmnb; - int msg_ctlmni; + unsigned int msg_ctlmax; + unsigned int msg_ctlmnb; + unsigned int msg_ctlmni; atomic_t msg_bytes; atomic_t msg_hdrs; int auto_msgmni; diff --git a/ipc/msgutil.c b/ipc/msgutil.c index 491e71f2a1b8..3cd9cb959c74 100644 --- a/ipc/msgutil.c +++ b/ipc/msgutil.c @@ -41,15 +41,15 @@ struct msg_msgseg { /* the next part of the message follows immediately */ }; -#define DATALEN_MSG (int)(PAGE_SIZE-sizeof(struct msg_msg)) -#define DATALEN_SEG (int)(PAGE_SIZE-sizeof(struct msg_msgseg)) +#define DATALEN_MSG (PAGE_SIZE-sizeof(struct msg_msg)) +#define DATALEN_SEG (PAGE_SIZE-sizeof(struct msg_msgseg)) -static struct msg_msg *alloc_msg(int len) +static struct msg_msg *alloc_msg(size_t len) { struct msg_msg *msg; struct msg_msgseg **pseg; - int alen; + size_t alen; alen = min(len, DATALEN_MSG); msg = kmalloc(sizeof(*msg) + alen, GFP_KERNEL); @@ -80,7 +80,7 @@ out_err: return NULL; } -struct msg_msg *load_msg(const void __user *src, int len) +struct msg_msg *load_msg(const void __user *src, size_t len) { struct msg_msg *msg; struct msg_msgseg *seg; @@ -147,9 +147,9 @@ struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg *dst) return ERR_PTR(-ENOSYS); } #endif -int store_msg(void __user *dest, struct msg_msg *msg, int len) +int store_msg(void __user *dest, struct msg_msg *msg, size_t len) { - int alen; + size_t alen; struct msg_msgseg *seg; alen = min(len, DATALEN_MSG); diff --git a/ipc/util.h b/ipc/util.h index f2f5036f2eed..59d78aa94987 100644 --- a/ipc/util.h +++ b/ipc/util.h @@ -148,9 +148,9 @@ int ipc_parse_version (int *cmd); #endif extern void free_msg(struct msg_msg *msg); -extern struct msg_msg *load_msg(const void __user *src, int len); +extern struct msg_msg *load_msg(const void __user *src, size_t len); extern struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg *dst); -extern int store_msg(void __user *dest, struct msg_msg *msg, int len); +extern int store_msg(void __user *dest, struct msg_msg *msg, size_t len); extern void recompute_msgmni(struct ipc_namespace *); | |