[PATCH] Drivers: hv: vmbus: annotate struct vmbus_gpadl with __counted_by_ptr

[PATCH] Drivers: hv: vmbus: annotate struct vmbus_gpadl with __counted_by_ptr

[Bill Wendling]

Add the "__counted_by_ptr" attribute to the buffer field of "struct
vmbus_gpadl". This allows compilers (GCC and Clang) to perform
compile-time and runtime bounds-checking when KASAN is enabled, preventing
potential out-of-bounds accesses to the GPADL buffer.

The fields "buffer" and "size" of "struct vmbus_gpadl" are assigned
exactly once, during GPADL establishment inside
"__vmbus_establish_gpadl()" in "drivers/hv/channel.c".

To ensure that the count field ("size") is initialized before the
pointer field ("buffer") is assigned, we reorder the assignments in
"__vmbus_establish_gpadl()" so that "gpadl->size" is written before
"gpadl->buffer".

Cc: codemender-patching+linux@google.com
Assisted-by: LLM
Signed-off-by: Bill Wendling <morbo@google.com>
---
 drivers/hv/channel.c   | 2 +-
 include/linux/hyperv.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 7e4cc6f55237..7042de2dd481 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -548,8 +548,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
 
 	/* At this point, we received the gpadl created msg */
 	gpadl->gpadl_handle = gpadlmsg->gpadl;
-	gpadl->buffer = kbuffer;
 	gpadl->size = size;
+	gpadl->buffer = kbuffer;
 
 
 cleanup:
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 9e109d91aa14..408748a05440 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -780,7 +780,7 @@ struct vmbus_device {
 struct vmbus_gpadl {
 	u32 gpadl_handle;
 	u32 size;
-	void *buffer;
+	void *buffer __counted_by_ptr(size);
 	bool decrypted;
 };
 
-- 
2.55.0.1082.g2b9226bbc0-goog

Re: [PATCH] Drivers: hv: vmbus: annotate struct vmbus_gpadl with __counted_by_ptr

[Gustavo A. R. Silva]

On 9/23/26 14:53, Bill Wendling wrote:
> Add the "__counted_by_ptr" attribute to the buffer field of "struct
> vmbus_gpadl". This allows compilers (GCC and Clang) to perform
> compile-time and runtime bounds-checking when KASAN is enabled, preventing
> potential out-of-bounds accesses to the GPADL buffer.
> 
> The fields "buffer" and "size" of "struct vmbus_gpadl" are assigned
> exactly once, during GPADL establishment inside
> "__vmbus_establish_gpadl()" in "drivers/hv/channel.c".
> 
> To ensure that the count field ("size") is initialized before the
> pointer field ("buffer") is assigned, we reorder the assignments in
> "__vmbus_establish_gpadl()" so that "gpadl->size" is written before
> "gpadl->buffer".
> 
> Cc: codemender-patching+linux@google.com
> Assisted-by: LLM
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
>   drivers/hv/channel.c   | 2 +-
>   include/linux/hyperv.h | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 7e4cc6f55237..7042de2dd481 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -548,8 +548,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>   
>   	/* At this point, we received the gpadl created msg */
>   	gpadl->gpadl_handle = gpadlmsg->gpadl;
> -	gpadl->buffer = kbuffer;
>   	gpadl->size = size;
> +	gpadl->buffer = kbuffer;

I think in some cases these subtle changes are worth a short but
informative comment saying that the _counter_ must be initialized
before the first reference to the pointer.

We can see this as part of the learning curve people have to go
through before the counted_by annotations and their requirements
are more widely known and understood. But I guess this can be
handled on a case-by-case basis.

Regardless:

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
-Gustavo

>   
>   
>   cleanup:
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 9e109d91aa14..408748a05440 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -780,7 +780,7 @@ struct vmbus_device {
>   struct vmbus_gpadl {
>   	u32 gpadl_handle;
>   	u32 size;
> -	void *buffer;
> +	void *buffer __counted_by_ptr(size);
>   	bool decrypted;
>   };
>   

Re: [PATCH] Drivers: hv: vmbus: annotate struct vmbus_gpadl with __counted_by_ptr

[Kees Cook]

On Wed, Sep 23, 2026 at 03:10:08PM +0900, Gustavo A. R. Silva wrote:
> 
> 
> On 9/23/26 14:53, Bill Wendling wrote:
> > Add the "__counted_by_ptr" attribute to the buffer field of "struct
> > vmbus_gpadl". This allows compilers (GCC and Clang) to perform
> > compile-time and runtime bounds-checking when KASAN is enabled, preventing
> > potential out-of-bounds accesses to the GPADL buffer.
> > 
> > The fields "buffer" and "size" of "struct vmbus_gpadl" are assigned
> > exactly once, during GPADL establishment inside
> > "__vmbus_establish_gpadl()" in "drivers/hv/channel.c".
> > 
> > To ensure that the count field ("size") is initialized before the
> > pointer field ("buffer") is assigned, we reorder the assignments in
> > "__vmbus_establish_gpadl()" so that "gpadl->size" is written before
> > "gpadl->buffer".
> > 
> > Cc: codemender-patching+linux@google.com
> > Assisted-by: LLM
> > Signed-off-by: Bill Wendling <morbo@google.com>
> > ---
> >   drivers/hv/channel.c   | 2 +-
> >   include/linux/hyperv.h | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> > index 7e4cc6f55237..7042de2dd481 100644
> > --- a/drivers/hv/channel.c
> > +++ b/drivers/hv/channel.c
> > @@ -548,8 +548,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
> >   	/* At this point, we received the gpadl created msg */
> >   	gpadl->gpadl_handle = gpadlmsg->gpadl;
> > -	gpadl->buffer = kbuffer;
> >   	gpadl->size = size;
> > +	gpadl->buffer = kbuffer;
> 
> I think in some cases these subtle changes are worth a short but
> informative comment saying that the _counter_ must be initialized
> before the first reference to the pointer.

But that's only for dereferencing it... there's no ordering requirement
here at all (and I think swapping order is needless churn). Neither
order is correct: only having them both set before dereferencing
"buffer" is required.


-- 
Kees Cook

Re: [PATCH] Drivers: hv: vmbus: annotate struct vmbus_gpadl with __counted_by_ptr

[Gustavo A. R. Silva]

On 9/23/26 16:23, Kees Cook wrote:
> On Wed, Sep 23, 2026 at 03:10:08PM +0900, Gustavo A. R. Silva wrote:
>>
>>
>> On 9/23/26 14:53, Bill Wendling wrote:
>>> Add the "__counted_by_ptr" attribute to the buffer field of "struct
>>> vmbus_gpadl". This allows compilers (GCC and Clang) to perform
>>> compile-time and runtime bounds-checking when KASAN is enabled, preventing
>>> potential out-of-bounds accesses to the GPADL buffer.
>>>
>>> The fields "buffer" and "size" of "struct vmbus_gpadl" are assigned
>>> exactly once, during GPADL establishment inside
>>> "__vmbus_establish_gpadl()" in "drivers/hv/channel.c".
>>>
>>> To ensure that the count field ("size") is initialized before the
>>> pointer field ("buffer") is assigned, we reorder the assignments in
>>> "__vmbus_establish_gpadl()" so that "gpadl->size" is written before
>>> "gpadl->buffer".
>>>
>>> Cc: codemender-patching+linux@google.com
>>> Assisted-by: LLM
>>> Signed-off-by: Bill Wendling <morbo@google.com>
>>> ---
>>>    drivers/hv/channel.c   | 2 +-
>>>    include/linux/hyperv.h | 2 +-
>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
>>> index 7e4cc6f55237..7042de2dd481 100644
>>> --- a/drivers/hv/channel.c
>>> +++ b/drivers/hv/channel.c
>>> @@ -548,8 +548,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>>>    	/* At this point, we received the gpadl created msg */
>>>    	gpadl->gpadl_handle = gpadlmsg->gpadl;
>>> -	gpadl->buffer = kbuffer;
>>>    	gpadl->size = size;
>>> +	gpadl->buffer = kbuffer;
>>
>> I think in some cases these subtle changes are worth a short but
>> informative comment saying that the _counter_ must be initialized
>> before the first reference to the pointer.
> 
> But that's only for dereferencing it... there's no ordering requirement

Ah yes, I got a bit carried away by the change itself.

> here at all (and I think swapping order is needless churn). Neither
> order is correct: only having them both set before dereferencing
> "buffer" is required.
Yes; maybe this is something the LLM should learn.

Thanks
-Gustavo