| Message ID | ZSRMosLuJJS5Y/io@work (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 492a84bd023127f0aaa25d4213aef2b11f9cec24 |
| Headers | show |
| Series | [next] xen/xenbus: Add __counted_by for struct read_buffer and use struct_size() | expand |
On Mon, Oct 09, 2023 at 12:55:30PM -0600, Gustavo A. R. Silva wrote: > Prepare for the coming implementation by GCC and Clang of the __counted_by > attribute. Flexible array members annotated with __counted_by can have > their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for > array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > functions). > > While there, use struct_size() helper, instead of the open-coded > version, to calculate the size for the allocation of the whole > flexible structure, including of course, the flexible-array member. > > This code was found with the help of Coccinelle, and audited and > fixed manually. > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Looks good. There are going to be lots of 1-byte flex array members... Reviewed-by: Kees Cook <keescook@chromium.org>
On Mon, Oct 9, 2023 at 2:55 PM Gustavo A. R. Silva <gustavoars@kernel.org> wrote: > > Prepare for the coming implementation by GCC and Clang of the __counted_by > attribute. Flexible array members annotated with __counted_by can have > their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for > array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > functions). > > While there, use struct_size() helper, instead of the open-coded > version, to calculate the size for the allocation of the whole > flexible structure, including of course, the flexible-array member. > > This code was found with the help of Coccinelle, and audited and > fixed manually. > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
diff --git a/drivers/xen/xenbus/xenbus_dev_frontend.c b/drivers/xen/xenbus/xenbus_dev_frontend.c index 0792fda49a15..6f56640092a9 100644 --- a/drivers/xen/xenbus/xenbus_dev_frontend.c +++ b/drivers/xen/xenbus/xenbus_dev_frontend.c @@ -82,7 +82,7 @@ struct read_buffer { struct list_head list; unsigned int cons; unsigned int len; - char msg[]; + char msg[] __counted_by(len); }; struct xenbus_file_priv { @@ -195,7 +195,7 @@ static int queue_reply(struct list_head *queue, const void *data, size_t len) if (len > XENSTORE_PAYLOAD_MAX) return -EINVAL; - rb = kmalloc(sizeof(*rb) + len, GFP_KERNEL); + rb = kmalloc(struct_size(rb, msg, len), GFP_KERNEL); if (rb == NULL) return -ENOMEM;
Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). While there, use struct_size() helper, instead of the open-coded version, to calculate the size for the allocation of the whole flexible structure, including of course, the flexible-array member. This code was found with the help of Coccinelle, and audited and fixed manually. Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/xen/xenbus/xenbus_dev_frontend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)