The vdpa_nl_policy structure is used to validate the nlattr when parsing
the incoming nlmsg. It will ensure the attribute being described produces
a valid nlattr pointer in info->attrs before entering into each handler
in vdpa_nl_ops.
That is to say, the missing part in vdpa_nl_policy may lead to illegal
nlattr after parsing, which could lead to OOB read just like CVE-2023-3773.
This patch adds three missing nla_policy to avoid such bugs.
Fixes: 90fea5a800c3 ("vdpa: device feature provisioning")
Fixes: 13b00b135665 ("vdpa: Add support for querying vendor statistics")
Fixes: ad69dd0bf26b ("vdpa: Introduce query of device config layout")
Signed-off-by: Lin Ma <[email protected]>
---
drivers/vdpa/vdpa.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 965e32529eb8..f2f654fd84e5 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -1247,8 +1247,11 @@ static const struct nla_policy vdpa_nl_policy[VDPA_ATTR_MAX + 1] = {
[VDPA_ATTR_MGMTDEV_DEV_NAME] = { .type = NLA_STRING },
[VDPA_ATTR_DEV_NAME] = { .type = NLA_STRING },
[VDPA_ATTR_DEV_NET_CFG_MACADDR] = NLA_POLICY_ETH_ADDR,
+ [VDPA_ATTR_DEV_NET_CFG_MAX_VQP] = { .type = NLA_U16 },
/* virtio spec 1.1 section 5.1.4.1 for valid MTU range */
[VDPA_ATTR_DEV_NET_CFG_MTU] = NLA_POLICY_MIN(NLA_U16, 68),
+ [VDPA_ATTR_DEV_QUEUE_INDEX] = { .type = NLA_U32 },
+ [VDPA_ATTR_DEV_FEATURES] = { .type = NLA_U64 },
};
static const struct genl_ops vdpa_nl_ops[] = {
--
2.17.1
On Sun, Jul 23, 2023 at 04:05:07PM +0800, Lin Ma wrote:
> The vdpa_nl_policy structure is used to validate the nlattr when parsing
> the incoming nlmsg. It will ensure the attribute being described produces
> a valid nlattr pointer in info->attrs before entering into each handler
> in vdpa_nl_ops.
>
> That is to say, the missing part in vdpa_nl_policy may lead to illegal
> nlattr after parsing, which could lead to OOB read just like CVE-2023-3773.
Hmm.
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-3773
** RESERVED ** This candidate has been reserved by an organization or individual that will use it when announcing a new security problem. When the candidate has been publicized, the details for this candidate will be provided.
> This patch adds three missing nla_policy to avoid such bugs.
>
> Fixes: 90fea5a800c3 ("vdpa: device feature provisioning")
> Fixes: 13b00b135665 ("vdpa: Add support for querying vendor statistics")
> Fixes: ad69dd0bf26b ("vdpa: Introduce query of device config layout")
> Signed-off-by: Lin Ma <[email protected]>
I don't know how OOB triggers but this duplication is problematic I
think: we are likely to forget again in the future. Isn't there a way
to block everything that is not listed?
> ---
> drivers/vdpa/vdpa.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 965e32529eb8..f2f654fd84e5 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -1247,8 +1247,11 @@ static const struct nla_policy vdpa_nl_policy[VDPA_ATTR_MAX + 1] = {
> [VDPA_ATTR_MGMTDEV_DEV_NAME] = { .type = NLA_STRING },
> [VDPA_ATTR_DEV_NAME] = { .type = NLA_STRING },
> [VDPA_ATTR_DEV_NET_CFG_MACADDR] = NLA_POLICY_ETH_ADDR,
> + [VDPA_ATTR_DEV_NET_CFG_MAX_VQP] = { .type = NLA_U16 },
> /* virtio spec 1.1 section 5.1.4.1 for valid MTU range */
> [VDPA_ATTR_DEV_NET_CFG_MTU] = NLA_POLICY_MIN(NLA_U16, 68),
> + [VDPA_ATTR_DEV_QUEUE_INDEX] = { .type = NLA_U32 },
> + [VDPA_ATTR_DEV_FEATURES] = { .type = NLA_U64 },
> };
>
> static const struct genl_ops vdpa_nl_ops[] = {
> --
> 2.17.1
Hello Michael,
> >
> > The vdpa_nl_policy structure is used to validate the nlattr when parsing
> > the incoming nlmsg. It will ensure the attribute being described produces
> > a valid nlattr pointer in info->attrs before entering into each handler
> > in vdpa_nl_ops.
> >
> > That is to say, the missing part in vdpa_nl_policy may lead to illegal
> > nlattr after parsing, which could lead to OOB read just like CVE-2023-3773.
>
> Hmm.
>
> https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-3773
>
> ** RESERVED ** This candidate has been reserved by an organization or individual that will use it when announcing a new security problem. When the candidate has been publicized, the details for this candidate will be provided.
>
Yeah, that CVE is assigned while fix not upstream yet. FYI, the fix is pending too.
See, https://marc.info/?l=linux-kernel&m=169009801131058&w=2.
>
> > This patch adds three missing nla_policy to avoid such bugs.
> >
> > Fixes: 90fea5a800c3 ("vdpa: device feature provisioning")
> > Fixes: 13b00b135665 ("vdpa: Add support for querying vendor statistics")
> > Fixes: ad69dd0bf26b ("vdpa: Introduce query of device config layout")
> > Signed-off-by: Lin Ma <[email protected]>
>
> I don't know how OOB triggers but this duplication is problematic I
> think: we are likely to forget again in the future. Isn't there a way
> to block everything that is not listed?
>
Sure, that is another undergoing task I'm working on. If the nlattr is parsed with
NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected, therefore (which is the default
for modern nla_parse). The problem here is that there are still consumers for
nla_parse_deprecated. And we cannot simply replace all *_deprecated to modern ones
as it may break userspace. See the commit message in 8cb081746c03 ("netlink: make
validation more configurable for future strictness")
I believe if we can do enough test against userspace toolchains, we can ultimately
upgrade all *_depprecated parsers to modern ones, which costs time and efforts. This
send patch is a much simpler (but temporary) solution for now.
Regards
Lin
> Sure, that is another undergoing task I'm working on. If the nlattr is parsed with
> NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected, therefore (which is the default
> for modern nla_parse).
For the general netlink interface, the deciding flag should be genl_ops.validate defined in
each ops. The default validate flag is strict, while the developer can overwrite the flag
with GENL_DONT_VALIDATE_STRICT to ease the validation. That is to say, safer code should
enforce NL_VALIDATE_STRICT by not overwriting the validate flag.
Regrads
Lin
On Sun, Jul 23, 2023 at 05:33:54PM +0800, Lin Ma wrote:
> Hello Michael,
>
> > >
> > > The vdpa_nl_policy structure is used to validate the nlattr when parsing
> > > the incoming nlmsg. It will ensure the attribute being described produces
> > > a valid nlattr pointer in info->attrs before entering into each handler
> > > in vdpa_nl_ops.
> > >
> > > That is to say, the missing part in vdpa_nl_policy may lead to illegal
> > > nlattr after parsing, which could lead to OOB read just like CVE-2023-3773.
> >
> > Hmm.
> >
> > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-3773
> >
> > ** RESERVED ** This candidate has been reserved by an organization or individual that will use it when announcing a new security problem. When the candidate has been publicized, the details for this candidate will be provided.
> >
>
> Yeah, that CVE is assigned while fix not upstream yet. FYI, the fix is pending too.
> See, https://marc.info/?l=linux-kernel&m=169009801131058&w=2.
>
> >
> > > This patch adds three missing nla_policy to avoid such bugs.
> > >
> > > Fixes: 90fea5a800c3 ("vdpa: device feature provisioning")
> > > Fixes: 13b00b135665 ("vdpa: Add support for querying vendor statistics")
> > > Fixes: ad69dd0bf26b ("vdpa: Introduce query of device config layout")
> > > Signed-off-by: Lin Ma <[email protected]>
> >
> > I don't know how OOB triggers but this duplication is problematic I
> > think: we are likely to forget again in the future. Isn't there a way
> > to block everything that is not listed?
> >
>
> Sure, that is another undergoing task I'm working on. If the nlattr is parsed with
> NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected, therefore (which is the default
> for modern nla_parse). The problem here is that there are still consumers for
> nla_parse_deprecated. And we cannot simply replace all *_deprecated to modern ones
> as it may break userspace. See the commit message in 8cb081746c03 ("netlink: make
> validation more configurable for future strictness")
>
> I believe if we can do enough test against userspace toolchains, we can ultimately
> upgrade all *_depprecated parsers to modern ones, which costs time and efforts. This
> send patch is a much simpler (but temporary) solution for now.
>
> Regards
> Lin
Hmm but vdpa does not use nla_parse_deprecated does it? And in fact was
introduced after 8cb081746c031fb164089322e2336a0bf5b3070c.
So why is there an issue in vdpa?
--
MST
On Sun, Jul 23, 2023 at 05:48:46PM +0800, Lin Ma wrote:
>
> > Sure, that is another undergoing task I'm working on. If the nlattr is parsed with
> > NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected, therefore (which is the default
> > for modern nla_parse).
>
> For the general netlink interface, the deciding flag should be genl_ops.validate defined in
> each ops. The default validate flag is strict, while the developer can overwrite the flag
> with GENL_DONT_VALIDATE_STRICT to ease the validation. That is to say, safer code should
> enforce NL_VALIDATE_STRICT by not overwriting the validate flag.
>
> Regrads
> Lin
Oh I see.
It started here:
commit 33b347503f014ebf76257327cbc7001c6b721956
Author: Parav Pandit <[email protected]>
Date: Tue Jan 5 12:32:00 2021 +0200
vdpa: Define vdpa mgmt device, ops and a netlink interface
which did:
+ .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
which was most likely just a copy paste from somewhere, right Parav?
and then everyone kept copying this around.
Parav, Eli can we drop these? There's a tiny chance of breaking something
but I feel there aren't that many users outside mlx5 yet, so if you
guys can test on mlx5 and confirm no breakage, I think we are good.
--
MST
On Sun, Jul 23, 2023 at 6:02 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Sun, Jul 23, 2023 at 05:48:46PM +0800, Lin Ma wrote:
> >
> > > Sure, that is another undergoing task I'm working on. If the nlattr is parsed with
> > > NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected, therefore (which is the default
> > > for modern nla_parse).
> >
> > For the general netlink interface, the deciding flag should be genl_ops.validate defined in
> > each ops. The default validate flag is strict, while the developer can overwrite the flag
> > with GENL_DONT_VALIDATE_STRICT to ease the validation. That is to say, safer code should
> > enforce NL_VALIDATE_STRICT by not overwriting the validate flag.
> >
> > Regrads
> > Lin
>
>
> Oh I see.
>
> It started here:
>
> commit 33b347503f014ebf76257327cbc7001c6b721956
> Author: Parav Pandit <[email protected]>
> Date: Tue Jan 5 12:32:00 2021 +0200
>
> vdpa: Define vdpa mgmt device, ops and a netlink interface
>
> which did:
>
> + .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
>
>
> which was most likely just a copy paste from somewhere, right Parav?
>
> and then everyone kept copying this around.
>
> Parav, Eli can we drop these? There's a tiny chance of breaking something
> but I feel there aren't that many users outside mlx5 yet, so if you
> guys can test on mlx5 and confirm no breakage, I think we are good.
Adding Dragos.
Thanks
>
> --
> MST
>
On Mon, 2023-07-24 at 15:11 +0800, Jason Wang wrote:
> On Sun, Jul 23, 2023 at 6:02 PM Michael S. Tsirkin <[email protected]> wrote:
> >
> > On Sun, Jul 23, 2023 at 05:48:46PM +0800, Lin Ma wrote:
> > >
> > > > Sure, that is another undergoing task I'm working on. If the nlattr is
> > > > parsed with
> > > > NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected, therefore
> > > > (which is the default
> > > > for modern nla_parse).
> > >
> > > For the general netlink interface, the deciding flag should be
> > > genl_ops.validate defined in
> > > each ops. The default validate flag is strict, while the developer can
> > > overwrite the flag
> > > with GENL_DONT_VALIDATE_STRICT to ease the validation. That is to say,
> > > safer code should
> > > enforce NL_VALIDATE_STRICT by not overwriting the validate flag.
> > >
> > > Regrads
> > > Lin
> >
> >
> > Oh I see.
> >
> > It started here:
> >
> > commit 33b347503f014ebf76257327cbc7001c6b721956
> > Author: Parav Pandit <[email protected]>
> > Date: Tue Jan 5 12:32:00 2021 +0200
> >
> > vdpa: Define vdpa mgmt device, ops and a netlink interface
> >
> > which did:
> >
> > + .validate = GENL_DONT_VALIDATE_STRICT |
> > GENL_DONT_VALIDATE_DUMP,
> >
> >
> > which was most likely just a copy paste from somewhere, right Parav?
> >
> > and then everyone kept copying this around.
> >
> > Parav, Eli can we drop these? There's a tiny chance of breaking something
> > but I feel there aren't that many users outside mlx5 yet, so if you
> > guys can test on mlx5 and confirm no breakage, I think we are good.
>
> Adding Dragos.
>
I will check. Just to make sure I understand correctly: you want me to drop the
.validate flags all together in all vdpa ops and check, right?
Thanks,
Dragos
On Mon, Jul 24, 2023 at 08:38:04AM +0000, Dragos Tatulea wrote:
>
> On Mon, 2023-07-24 at 15:11 +0800, Jason Wang wrote:
> > On Sun, Jul 23, 2023 at 6:02 PM Michael S. Tsirkin <[email protected]> wrote:
> > >
> > > On Sun, Jul 23, 2023 at 05:48:46PM +0800, Lin Ma wrote:
> > > >
> > > > > Sure, that is another undergoing task I'm working on. If the nlattr is
> > > > > parsed with
> > > > > NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected, therefore
> > > > > (which is the default
> > > > > for modern nla_parse).
> > > >
> > > > For the general netlink interface, the deciding flag should be
> > > > genl_ops.validate defined in
> > > > each ops. The default validate flag is strict, while the developer can
> > > > overwrite the flag
> > > > with GENL_DONT_VALIDATE_STRICT to ease the validation. That is to say,
> > > > safer code should
> > > > enforce NL_VALIDATE_STRICT by not overwriting the validate flag.
> > > >
> > > > Regrads
> > > > Lin
> > >
> > >
> > > Oh I see.
> > >
> > > It started here:
> > >
> > > commit 33b347503f014ebf76257327cbc7001c6b721956
> > > Author: Parav Pandit <[email protected]>
> > > Date: Tue Jan 5 12:32:00 2021 +0200
> > >
> > > vdpa: Define vdpa mgmt device, ops and a netlink interface
> > >
> > > which did:
> > >
> > > + .validate = GENL_DONT_VALIDATE_STRICT |
> > > GENL_DONT_VALIDATE_DUMP,
> > >
> > >
> > > which was most likely just a copy paste from somewhere, right Parav?
> > >
> > > and then everyone kept copying this around.
> > >
> > > Parav, Eli can we drop these? There's a tiny chance of breaking something
> > > but I feel there aren't that many users outside mlx5 yet, so if you
> > > guys can test on mlx5 and confirm no breakage, I think we are good.
> >
> > Adding Dragos.
> >
> I will check. Just to make sure I understand correctly: you want me to drop the
> .validate flags all together in all vdpa ops and check, right?
>
> Thanks,
> Dragos
yes - I suspect you will then need this patch to make things work.
--
MST
On Mon, 2023-07-24 at 05:16 -0400, Michael S. Tsirkin wrote:
> On Mon, Jul 24, 2023 at 08:38:04AM +0000, Dragos Tatulea wrote:
> >
> > On Mon, 2023-07-24 at 15:11 +0800, Jason Wang wrote:
> > > On Sun, Jul 23, 2023 at 6:02 PM Michael S. Tsirkin <[email protected]> wrote:
> > > >
> > > > On Sun, Jul 23, 2023 at 05:48:46PM +0800, Lin Ma wrote:
> > > > >
> > > > > > Sure, that is another undergoing task I'm working on. If the nlattr
> > > > > > is
> > > > > > parsed with
> > > > > > NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected, therefore
> > > > > > (which is the default
> > > > > > for modern nla_parse).
> > > > >
> > > > > For the general netlink interface, the deciding flag should be
> > > > > genl_ops.validate defined in
> > > > > each ops. The default validate flag is strict, while the developer can
> > > > > overwrite the flag
> > > > > with GENL_DONT_VALIDATE_STRICT to ease the validation. That is to say,
> > > > > safer code should
> > > > > enforce NL_VALIDATE_STRICT by not overwriting the validate flag.
> > > > >
> > > > > Regrads
> > > > > Lin
> > > >
> > > >
> > > > Oh I see.
> > > >
> > > > It started here:
> > > >
> > > > commit 33b347503f014ebf76257327cbc7001c6b721956
> > > > Author: Parav Pandit <[email protected]>
> > > > Date: Tue Jan 5 12:32:00 2021 +0200
> > > >
> > > > vdpa: Define vdpa mgmt device, ops and a netlink interface
> > > >
> > > > which did:
> > > >
> > > > + .validate = GENL_DONT_VALIDATE_STRICT |
> > > > GENL_DONT_VALIDATE_DUMP,
> > > >
> > > >
> > > > which was most likely just a copy paste from somewhere, right Parav?
> > > >
> > > > and then everyone kept copying this around.
> > > >
> > > > Parav, Eli can we drop these? There's a tiny chance of breaking
> > > > something
> > > > but I feel there aren't that many users outside mlx5 yet, so if you
> > > > guys can test on mlx5 and confirm no breakage, I think we are good.
> > >
> > > Adding Dragos.
> > >
> > I will check. Just to make sure I understand correctly: you want me to drop
> > the
> > .validate flags all together in all vdpa ops and check, right?
> >
> > Thanks,
> > Dragos
>
> yes - I suspect you will then need this patch to make things work.
>
Yep. Adding the patch and removing the .validate config on the vdpa_nl_ops
seems to work just fine.
Thanks,
Dragos
On Mon, Jul 24, 2023 at 11:42:42AM +0000, Dragos Tatulea wrote:
> On Mon, 2023-07-24 at 05:16 -0400, Michael S. Tsirkin wrote:
> > On Mon, Jul 24, 2023 at 08:38:04AM +0000, Dragos Tatulea wrote:
> > >
> > > On Mon, 2023-07-24 at 15:11 +0800, Jason Wang wrote:
> > > > On Sun, Jul 23, 2023 at 6:02 PM Michael S. Tsirkin <[email protected]> wrote:
> > > > >
> > > > > On Sun, Jul 23, 2023 at 05:48:46PM +0800, Lin Ma wrote:
> > > > > >
> > > > > > > Sure, that is another undergoing task I'm working on. If the nlattr
> > > > > > > is
> > > > > > > parsed with
> > > > > > > NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected, therefore
> > > > > > > (which is the default
> > > > > > > for modern nla_parse).
> > > > > >
> > > > > > For the general netlink interface, the deciding flag should be
> > > > > > genl_ops.validate defined in
> > > > > > each ops. The default validate flag is strict, while the developer can
> > > > > > overwrite the flag
> > > > > > with GENL_DONT_VALIDATE_STRICT to ease the validation. That is to say,
> > > > > > safer code should
> > > > > > enforce NL_VALIDATE_STRICT by not overwriting the validate flag.
> > > > > >
> > > > > > Regrads
> > > > > > Lin
> > > > >
> > > > >
> > > > > Oh I see.
> > > > >
> > > > > It started here:
> > > > >
> > > > > commit 33b347503f014ebf76257327cbc7001c6b721956
> > > > > Author: Parav Pandit <[email protected]>
> > > > > Date: Tue Jan 5 12:32:00 2021 +0200
> > > > >
> > > > > vdpa: Define vdpa mgmt device, ops and a netlink interface
> > > > >
> > > > > which did:
> > > > >
> > > > > + .validate = GENL_DONT_VALIDATE_STRICT |
> > > > > GENL_DONT_VALIDATE_DUMP,
> > > > >
> > > > >
> > > > > which was most likely just a copy paste from somewhere, right Parav?
> > > > >
> > > > > and then everyone kept copying this around.
> > > > >
> > > > > Parav, Eli can we drop these? There's a tiny chance of breaking
> > > > > something
> > > > > but I feel there aren't that many users outside mlx5 yet, so if you
> > > > > guys can test on mlx5 and confirm no breakage, I think we are good.
> > > >
> > > > Adding Dragos.
> > > >
> > > I will check. Just to make sure I understand correctly: you want me to drop
> > > the
> > > .validate flags all together in all vdpa ops and check, right?
> > >
> > > Thanks,
> > > Dragos
> >
> > yes - I suspect you will then need this patch to make things work.
> >
> Yep. Adding the patch and removing the .validate config on the vdpa_nl_ops
> seems to work just fine.
>
> Thanks,
> Dragos
OK, post a patch?
--
MST
On Mon, 2023-07-24 at 16:08 -0400, Michael S. Tsirkin wrote:
> On Mon, Jul 24, 2023 at 11:42:42AM +0000, Dragos Tatulea wrote:
> > On Mon, 2023-07-24 at 05:16 -0400, Michael S. Tsirkin wrote:
> > > On Mon, Jul 24, 2023 at 08:38:04AM +0000, Dragos Tatulea wrote:
> > > >
> > > > On Mon, 2023-07-24 at 15:11 +0800, Jason Wang wrote:
> > > > > On Sun, Jul 23, 2023 at 6:02 PM Michael S. Tsirkin <[email protected]>
> > > > > wrote:
> > > > > >
> > > > > > On Sun, Jul 23, 2023 at 05:48:46PM +0800, Lin Ma wrote:
> > > > > > >
> > > > > > > > Sure, that is another undergoing task I'm working on. If the
> > > > > > > > nlattr
> > > > > > > > is
> > > > > > > > parsed with
> > > > > > > > NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected,
> > > > > > > > therefore
> > > > > > > > (which is the default
> > > > > > > > for modern nla_parse).
> > > > > > >
> > > > > > > For the general netlink interface, the deciding flag should be
> > > > > > > genl_ops.validate defined in
> > > > > > > each ops. The default validate flag is strict, while the developer
> > > > > > > can
> > > > > > > overwrite the flag
> > > > > > > with GENL_DONT_VALIDATE_STRICT to ease the validation. That is to
> > > > > > > say,
> > > > > > > safer code should
> > > > > > > enforce NL_VALIDATE_STRICT by not overwriting the validate flag.
> > > > > > >
> > > > > > > Regrads
> > > > > > > Lin
> > > > > >
> > > > > >
> > > > > > Oh I see.
> > > > > >
> > > > > > It started here:
> > > > > >
> > > > > > commit 33b347503f014ebf76257327cbc7001c6b721956
> > > > > > Author: Parav Pandit <[email protected]>
> > > > > > Date: Tue Jan 5 12:32:00 2021 +0200
> > > > > >
> > > > > > vdpa: Define vdpa mgmt device, ops and a netlink interface
> > > > > >
> > > > > > which did:
> > > > > >
> > > > > > + .validate = GENL_DONT_VALIDATE_STRICT |
> > > > > > GENL_DONT_VALIDATE_DUMP,
> > > > > >
> > > > > >
> > > > > > which was most likely just a copy paste from somewhere, right Parav?
> > > > > >
> > > > > > and then everyone kept copying this around.
> > > > > >
> > > > > > Parav, Eli can we drop these? There's a tiny chance of breaking
> > > > > > something
> > > > > > but I feel there aren't that many users outside mlx5 yet, so if you
> > > > > > guys can test on mlx5 and confirm no breakage, I think we are good.
> > > > >
> > > > > Adding Dragos.
> > > > >
> > > > I will check. Just to make sure I understand correctly: you want me to
> > > > drop
> > > > the
> > > > .validate flags all together in all vdpa ops and check, right?
> > > >
> > > > Thanks,
> > > > Dragos
> > >
> > > yes - I suspect you will then need this patch to make things work.
> > >
> > Yep. Adding the patch and removing the .validate config on the vdpa_nl_ops
> > seems to work just fine.
> >
> > Thanks,
> > Dragos
>
> OK, post a patch?
>
Sure, but how do I make it depend on this patch? Otherwise it will break things.
Thanks,
Dragos
On Tue, Jul 25, 2023 at 08:26:32AM +0000, Dragos Tatulea wrote:
> On Mon, 2023-07-24 at 16:08 -0400, Michael S. Tsirkin wrote:
> > On Mon, Jul 24, 2023 at 11:42:42AM +0000, Dragos Tatulea wrote:
> > > On Mon, 2023-07-24 at 05:16 -0400, Michael S. Tsirkin wrote:
> > > > On Mon, Jul 24, 2023 at 08:38:04AM +0000, Dragos Tatulea wrote:
> > > > >
> > > > > On Mon, 2023-07-24 at 15:11 +0800, Jason Wang wrote:
> > > > > > On Sun, Jul 23, 2023 at 6:02 PM Michael S. Tsirkin <[email protected]>
> > > > > > wrote:
> > > > > > >
> > > > > > > On Sun, Jul 23, 2023 at 05:48:46PM +0800, Lin Ma wrote:
> > > > > > > >
> > > > > > > > > Sure, that is another undergoing task I'm working on. If the
> > > > > > > > > nlattr
> > > > > > > > > is
> > > > > > > > > parsed with
> > > > > > > > > NL_VALIDATE_UNSPEC, any forgotten nlattr will be rejected,
> > > > > > > > > therefore
> > > > > > > > > (which is the default
> > > > > > > > > for modern nla_parse).
> > > > > > > >
> > > > > > > > For the general netlink interface, the deciding flag should be
> > > > > > > > genl_ops.validate defined in
> > > > > > > > each ops. The default validate flag is strict, while the developer
> > > > > > > > can
> > > > > > > > overwrite the flag
> > > > > > > > with GENL_DONT_VALIDATE_STRICT to ease the validation. That is to
> > > > > > > > say,
> > > > > > > > safer code should
> > > > > > > > enforce NL_VALIDATE_STRICT by not overwriting the validate flag.
> > > > > > > >
> > > > > > > > Regrads
> > > > > > > > Lin
> > > > > > >
> > > > > > >
> > > > > > > Oh I see.
> > > > > > >
> > > > > > > It started here:
> > > > > > >
> > > > > > > commit 33b347503f014ebf76257327cbc7001c6b721956
> > > > > > > Author: Parav Pandit <[email protected]>
> > > > > > > Date: Tue Jan 5 12:32:00 2021 +0200
> > > > > > >
> > > > > > > vdpa: Define vdpa mgmt device, ops and a netlink interface
> > > > > > >
> > > > > > > which did:
> > > > > > >
> > > > > > > + .validate = GENL_DONT_VALIDATE_STRICT |
> > > > > > > GENL_DONT_VALIDATE_DUMP,
> > > > > > >
> > > > > > >
> > > > > > > which was most likely just a copy paste from somewhere, right Parav?
> > > > > > >
> > > > > > > and then everyone kept copying this around.
> > > > > > >
> > > > > > > Parav, Eli can we drop these? There's a tiny chance of breaking
> > > > > > > something
> > > > > > > but I feel there aren't that many users outside mlx5 yet, so if you
> > > > > > > guys can test on mlx5 and confirm no breakage, I think we are good.
> > > > > >
> > > > > > Adding Dragos.
> > > > > >
> > > > > I will check. Just to make sure I understand correctly: you want me to
> > > > > drop
> > > > > the
> > > > > .validate flags all together in all vdpa ops and check, right?
> > > > >
> > > > > Thanks,
> > > > > Dragos
> > > >
> > > > yes - I suspect you will then need this patch to make things work.
> > > >
> > > Yep. Adding the patch and removing the .validate config on the vdpa_nl_ops
> > > seems to work just fine.
> > >
> > > Thanks,
> > > Dragos
> >
> > OK, post a patch?
> >
> Sure, but how do I make it depend on this patch? Otherwise it will break things.
>
> Thanks,
> Dragos
Send a patch series with this as patch 1/2 that one 2/2.