[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

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

[Michael Kelley]

From: Bill Wendling <morbo@google.com> Sent: Tuesday, September 22, 2026 10:53 PM
> 
> 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.

It turns out this "buffer" field is never dereferenced. The value is saved
here only for the purpose of passing into set_memory_encrypted() as
called by vmbus_teardown_gpadl(). The memory *is* referenced, but
through a wraparound mapping created by vmap() from the struct page's
underlying the buffer. The mapping is created in hv_ringbuffer_init().

Given that, is this patch still appropriate?

Michael

> 
> 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

[Bill Wendling]

On Fri, Sep 25, 2026 at 6:55 PM Michael Kelley <mhklinux@outlook.com> wrote:
>
> From: Bill Wendling <morbo@google.com> Sent: Tuesday, September 22, 2026 10:53 PM
> >
> > 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.
>
> It turns out this "buffer" field is never dereferenced. The value is saved
> here only for the purpose of passing into set_memory_encrypted() as
> called by vmbus_teardown_gpadl(). The memory *is* referenced, but
> through a wraparound mapping created by vmap() from the struct page's
> underlying the buffer. The mapping is created in hv_ringbuffer_init().
>
> Given that, is this patch still appropriate?
>
Hi Michael,

Huh...so it is. The buffer is converted to 'unsigned long' (*sigh*)
and then passed into arch-specific functions, which do what you
mentioned above. Given that please feel free to ignore this patch.
Apologies for the extra emails...

-bw

> > 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

[Bill Wendling]

On Wed, Sep 23, 2026 at 1:02 AM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
> 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

I've explicitly told the LLM exactly this, but it either forgets or
ignores it. *sigh*

-bw