On Wed, Oct 25, 2023 at 10:11:41PM +0900, Namjae Jeon wrote: > > @@ -757,7 +756,7 @@ struct ksmbd_rpc_command *ksmbd_rpc_ioctl(struct > Hi Dan, > > > ksmbd_session *sess, int handle > > struct ksmbd_rpc_command *req; > > struct ksmbd_rpc_command *resp; > > > > - msg = ipc_msg_alloc(sizeof(struct ksmbd_rpc_command) + payload_sz + 1); > > + msg = ipc_msg_alloc(size_add(sizeof(struct ksmbd_rpc_command) + 1, > > payload_sz)); > > if (!msg) > > return NULL; > There is a memcpy() below as follows. > memcpy(req->payload, payload, payload_sz); > > Doesn't memcpy with payload_sz cause buffer overflow? > Wouldn't it be better to handle integer overflows as an error? In the original code, then the memcpy() is the issue I was concerned about. I don't think it can be easily used for privelege escalation but it can definitly crash the system. The danger over integer overflows is that you do some math and the size becomes small and the allocation succeeds. But then we do an memcpy() with no math and the size is very large. We're trying to memcpy() a large thing into a small buffer and it overflows. What size_add() does is that if there is an integer overflow then instead of wrapping to a small number then it just gets stuck at ULONG_MAX. The allocation will fail. The allocation will actually fail pretty quickly but even if it didn't that's fine because we optimize for the success case (and not for hackers). The rules with size_add() are that you have to be careful with the results. You can't add use it for math again except through the size_add/mul() helpers. You also must always save the result to size_t or unsigned long. regards, dan carpenter
