[PATCH] vDPA: fix incorrect VDPA_ATTR_MAX value

[PATCH] vDPA: fix incorrect VDPA_ATTR_MAX value

[kovalev]

From: Vasiliy Kovalev <kovalev@altlinux.org>

The VDPA_ATTR_MAX value should correspond to the index of the last
available member of the structure, not their total number.

Otherwise, it can lead to interpretation errors in other functions
when the structure (.maxattr = VDPA_ATTR_MAX) member is actually
incremented by one and refers to invalid data.

Fixes: 33b347503f01 ("vdpa: Define vdpa mgmt device, ops and a netlink interface")
Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
---
 include/uapi/linux/vdpa.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 43c51698195ceb..ab132a09565232 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -74,7 +74,8 @@ enum vdpa_attr {
 	VDPA_ATTR_DEV_BLK_CFG_FLUSH,		/* u8 */
 
 	/* new attributes must be added above here */
-	VDPA_ATTR_MAX,
+	__VDPA_ATTR_MAX,
+	VDPA_ATTR_MAX	= __VDPA_ATTR_MAX - 1,
 };
 
 #endif
-- 
2.33.8

Re: [PATCH] vDPA: fix incorrect VDPA_ATTR_MAX value

[Jason Wang]

On Tue, Apr 16, 2024 at 12:39 AM <kovalev@altlinux.org> wrote:
>
> From: Vasiliy Kovalev <kovalev@altlinux.org>
>
> The VDPA_ATTR_MAX value should correspond to the index of the last
> available member of the structure, not their total number.

I think it's too late to change. More below.

>
> Otherwise, it can lead to interpretation errors in other functions
> when the structure (.maxattr = VDPA_ATTR_MAX) member is actually
> incremented by one and refers to invalid data.
>
> Fixes: 33b347503f01 ("vdpa: Define vdpa mgmt device, ops and a netlink interface")
> Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
> ---
>  include/uapi/linux/vdpa.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> index 43c51698195ceb..ab132a09565232 100644
> --- a/include/uapi/linux/vdpa.h
> +++ b/include/uapi/linux/vdpa.h
> @@ -74,7 +74,8 @@ enum vdpa_attr {
>         VDPA_ATTR_DEV_BLK_CFG_FLUSH,            /* u8 */
>
>         /* new attributes must be added above here */
> -       VDPA_ATTR_MAX,
> +       __VDPA_ATTR_MAX,
> +       VDPA_ATTR_MAX   = __VDPA_ATTR_MAX - 1,

Having a MAX in uapi is problematic as it may confuse the userspace. I
think the correct way is to drop it from uAPI.

Thanks

>  };
>
>  #endif
> --
> 2.33.8
>
>

Re: [PATCH] vDPA: fix incorrect VDPA_ATTR_MAX value

[Jason Wang]

On Tue, Apr 16, 2024 at 3:18 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Apr 16, 2024 at 12:39 AM <kovalev@altlinux.org> wrote:
> >
> > From: Vasiliy Kovalev <kovalev@altlinux.org>
> >
> > The VDPA_ATTR_MAX value should correspond to the index of the last
> > available member of the structure, not their total number.
>
> I think it's too late to change. More below.
>
> >
> > Otherwise, it can lead to interpretation errors in other functions
> > when the structure (.maxattr = VDPA_ATTR_MAX) member is actually
> > incremented by one and refers to invalid data.
> >
> > Fixes: 33b347503f01 ("vdpa: Define vdpa mgmt device, ops and a netlink interface")
> > Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
> > ---
> >  include/uapi/linux/vdpa.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> > index 43c51698195ceb..ab132a09565232 100644
> > --- a/include/uapi/linux/vdpa.h
> > +++ b/include/uapi/linux/vdpa.h
> > @@ -74,7 +74,8 @@ enum vdpa_attr {
> >         VDPA_ATTR_DEV_BLK_CFG_FLUSH,            /* u8 */
> >
> >         /* new attributes must be added above here */
> > -       VDPA_ATTR_MAX,
> > +       __VDPA_ATTR_MAX,
> > +       VDPA_ATTR_MAX   = __VDPA_ATTR_MAX - 1,
>
> Having a MAX in uapi is problematic as it may confuse the userspace. I
> think the correct way is to drop it from uAPI.

Too late to drop as well probably ...

Thanks

>
> Thanks
>
> >  };
> >
> >  #endif
> > --
> > 2.33.8
> >
> >