From: Vasiliy Kovalev <[email protected]>
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 <[email protected]>
---
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
On Tue, Apr 16, 2024 at 12:39 AM <[email protected]> wrote:
>
> From: Vasiliy Kovalev <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
>
On Tue, Apr 16, 2024 at 3:18 PM Jason Wang <[email protected]> wrote:
>
> On Tue, Apr 16, 2024 at 12:39 AM <[email protected]> wrote:
> >
> > From: Vasiliy Kovalev <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
> >
> >