| Message ID | 1500860146-26970-2-git-send-email-jhs@emojatatu.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Mon, Jul 24, 2017 at 03:35:43AM CEST, jhs@mojatatu.com wrote: >From: Jamal Hadi Salim <jhs@mojatatu.com> > >Generic bitflags attribute content sent to the kernel by user. >With this type the user can either set or unset a flag in the >kernel. > >The nla_value is a bitmap that defines the values being set >The nla_selector is a bitmask that defines which value is legit. > >A check is made to ensure the rules that a kernel subsystem always >conforms to bitflags the kernel already knows about. i.e >if the user tries to set a bit flag that is not understood then >the _it will be rejected_. > >In the most basic form, the user specifies the attribute policy as: >[ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = &myvalidflags }, > >where myvalidflags is the bit mask of the flags the kernel understands. > >If the user _does not_ provide myvalidflags then the attribute will >also be rejected. > >Examples: >nla_value = 0x0, and nla_selector = 0x1 >implies we are selecting bit 1 and we want to set its value to 0. > >nla_value = 0x2, and nla_selector = 0x2 >implies we are selecting bit 2 and we want to set its value to 1. > >Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> >--- > include/net/netlink.h | 4 ++++ > include/uapi/linux/netlink.h | 17 +++++++++++++++++ > lib/nlattr.c | 21 +++++++++++++++++++++ > 3 files changed, 42 insertions(+) > >diff --git a/include/net/netlink.h b/include/net/netlink.h >index ef8e6c3..e33d1fb 100644 >--- a/include/net/netlink.h >+++ b/include/net/netlink.h >@@ -178,6 +178,7 @@ enum { > NLA_S16, > NLA_S32, > NLA_S64, >+ NLA_BITFIELD_32, > __NLA_TYPE_MAX, > }; > >@@ -206,6 +207,7 @@ enum { > * NLA_MSECS Leaving the length field zero will verify the > * given type fits, using it verifies minimum length > * just like "All other" >+ * NLA_BITFIELD_32 A 32-bit bitmap/bitselector attribute > * All other Minimum length of attribute payload > * > * Example: >@@ -213,11 +215,13 @@ enum { > * [ATTR_FOO] = { .type = NLA_U16 }, > * [ATTR_BAR] = { .type = NLA_STRING, .len = BARSIZ }, > * [ATTR_BAZ] = { .len = sizeof(struct mystruct) }, >+ * [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = &myvalidflags }, > * }; > */ > struct nla_policy { > u16 type; > u16 len; >+ void *validation_data; > }; > > /** >diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h >index f86127a..0ac05a6 100644 >--- a/include/uapi/linux/netlink.h >+++ b/include/uapi/linux/netlink.h >@@ -226,5 +226,22 @@ struct nlattr { > #define NLA_ALIGN(len) (((len) + NLA_ALIGNTO - 1) & ~(NLA_ALIGNTO - 1)) > #define NLA_HDRLEN ((int) NLA_ALIGN(sizeof(struct nlattr))) > >+/* Generic 32 bitflags attribute content sent to the kernel. >+ * >+ * The nla_value is a bitmap that defines the values being set >+ * The nla_selector is a bitmask that defines which value is legit >+ * >+ * Examples: >+ * nla_value = 0x0, and nla_selector = 0x1 >+ * implies we are selecting bit 1 and we want to set its value to 0. >+ * >+ * nla_value = 0x2, and nla_selector = 0x2 >+ * implies we are selecting bit 2 and we want to set its value to 1. >+ * >+ */ >+struct nla_bitfield_32 { >+ __u32 nla_value; >+ __u32 nla_selector; I would just have "value" and "selector" here. "nla_" prefix indicates netlink attrubute, like in struct nlattr - nla_type, nla_len, and that is wrong indication. >+}; > > #endif /* _UAPI__LINUX_NETLINK_H */ >diff --git a/lib/nlattr.c b/lib/nlattr.c >index fb52435..c8aad7e 100644 >--- a/lib/nlattr.c >+++ b/lib/nlattr.c >@@ -27,6 +27,20 @@ > [NLA_S64] = sizeof(s64), > }; > >+static int validate_nla_bitfield_32(const struct nlattr *nla, void *valid_data) This should be: static int validate_nla_bitfield_32(const struct nlattr *nla, u32 *valid_flags_allowed) Other than this 2 nits, this looks good. >+{ >+ const struct nla_bitfield_32 *nbf = nla_data(nla); >+ u32 *valid_flags_mask = valid_data; >+ >+ if (!valid_data) >+ return -EINVAL; >+ >+ if (nbf->nla_value & ~*valid_flags_mask) >+ return -EINVAL; >+ >+ return 0; >+} >+ > static int validate_nla(const struct nlattr *nla, int maxtype, > const struct nla_policy *policy) > { >@@ -46,6 +60,13 @@ static int validate_nla(const struct nlattr *nla, int maxtype, > return -ERANGE; > break; > >+ case NLA_BITFIELD_32: >+ if (attrlen != sizeof(struct nla_bitfield_32)) >+ return -ERANGE; >+ >+ return validate_nla_bitfield_32(nla, pt->validation_data); >+ break; >+ > case NLA_NUL_STRING: > if (pt->len) > minlen = min_t(int, attrlen, pt->len + 1); >-- >1.9.1 >
Mon, Jul 24, 2017 at 03:35:43AM CEST, jhs@mojatatu.com wrote: >From: Jamal Hadi Salim <jhs@mojatatu.com> > >Generic bitflags attribute content sent to the kernel by user. >With this type the user can either set or unset a flag in the >kernel. > >The nla_value is a bitmap that defines the values being set >The nla_selector is a bitmask that defines which value is legit. > >A check is made to ensure the rules that a kernel subsystem always >conforms to bitflags the kernel already knows about. i.e >if the user tries to set a bit flag that is not understood then >the _it will be rejected_. > >In the most basic form, the user specifies the attribute policy as: >[ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = &myvalidflags }, > >where myvalidflags is the bit mask of the flags the kernel understands. > >If the user _does not_ provide myvalidflags then the attribute will >also be rejected. > >Examples: >nla_value = 0x0, and nla_selector = 0x1 >implies we are selecting bit 1 and we want to set its value to 0. > >nla_value = 0x2, and nla_selector = 0x2 >implies we are selecting bit 2 and we want to set its value to 1. Oh, 2 more things: [...] >@@ -46,6 +60,13 @@ static int validate_nla(const struct nlattr *nla, int maxtype, > return -ERANGE; > break; > >+ case NLA_BITFIELD_32: Now that I'm looking at it, perhaps just "NLA_BITFIELD32" looks nicer and aligns with "NLA_U32" and others. >+ if (attrlen != sizeof(struct nla_bitfield_32)) >+ return -ERANGE; >+ >+ return validate_nla_bitfield_32(nla, pt->validation_data); >+ break; Remove the pointless "break" from here.
On 17-07-24 07:14 AM, Jiri Pirko wrote: > Mon, Jul 24, 2017 at 03:35:43AM CEST, jhs@mojatatu.com wrote: >> From: Jamal Hadi Salim <jhs@mojatatu.com> >> >> Generic bitflags attribute content sent to the kernel by user. >> With this type the user can either set or unset a flag in the >> kernel. >> >> The nla_value is a bitmap that defines the values being set >> The nla_selector is a bitmask that defines which value is legit. >> >> A check is made to ensure the rules that a kernel subsystem always >> conforms to bitflags the kernel already knows about. i.e >> if the user tries to set a bit flag that is not understood then >> the _it will be rejected_. >> >> In the most basic form, the user specifies the attribute policy as: >> [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = &myvalidflags }, >> >> where myvalidflags is the bit mask of the flags the kernel understands. >> >> If the user _does not_ provide myvalidflags then the attribute will >> also be rejected. >> >> Examples: >> nla_value = 0x0, and nla_selector = 0x1 >> implies we are selecting bit 1 and we want to set its value to 0. >> >> nla_value = 0x2, and nla_selector = 0x2 >> implies we are selecting bit 2 and we want to set its value to 1. >> >> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> >> --- >> include/net/netlink.h | 4 ++++ >> include/uapi/linux/netlink.h | 17 +++++++++++++++++ >> lib/nlattr.c | 21 +++++++++++++++++++++ >> 3 files changed, 42 insertions(+) >> >> diff --git a/include/net/netlink.h b/include/net/netlink.h >> index ef8e6c3..e33d1fb 100644 >> --- a/include/net/netlink.h >> +++ b/include/net/netlink.h >> @@ -178,6 +178,7 @@ enum { >> NLA_S16, >> NLA_S32, >> NLA_S64, >> + NLA_BITFIELD_32, >> __NLA_TYPE_MAX, >> }; >> >> @@ -206,6 +207,7 @@ enum { >> * NLA_MSECS Leaving the length field zero will verify the >> * given type fits, using it verifies minimum length >> * just like "All other" >> + * NLA_BITFIELD_32 A 32-bit bitmap/bitselector attribute >> * All other Minimum length of attribute payload >> * >> * Example: >> @@ -213,11 +215,13 @@ enum { >> * [ATTR_FOO] = { .type = NLA_U16 }, >> * [ATTR_BAR] = { .type = NLA_STRING, .len = BARSIZ }, >> * [ATTR_BAZ] = { .len = sizeof(struct mystruct) }, >> + * [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = &myvalidflags }, >> * }; >> */ >> struct nla_policy { >> u16 type; >> u16 len; >> + void *validation_data; >> }; >> >> /** >> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h >> index f86127a..0ac05a6 100644 >> --- a/include/uapi/linux/netlink.h >> +++ b/include/uapi/linux/netlink.h >> @@ -226,5 +226,22 @@ struct nlattr { >> #define NLA_ALIGN(len) (((len) + NLA_ALIGNTO - 1) & ~(NLA_ALIGNTO - 1)) >> #define NLA_HDRLEN ((int) NLA_ALIGN(sizeof(struct nlattr))) >> >> +/* Generic 32 bitflags attribute content sent to the kernel. >> + * >> + * The nla_value is a bitmap that defines the values being set >> + * The nla_selector is a bitmask that defines which value is legit >> + * >> + * Examples: >> + * nla_value = 0x0, and nla_selector = 0x1 >> + * implies we are selecting bit 1 and we want to set its value to 0. >> + * >> + * nla_value = 0x2, and nla_selector = 0x2 >> + * implies we are selecting bit 2 and we want to set its value to 1. >> + * >> + */ >> +struct nla_bitfield_32 { >> + __u32 nla_value; >> + __u32 nla_selector; > > I would just have "value" and "selector" here. "nla_" prefix indicates > netlink attrubute, like in struct nlattr - nla_type, nla_len, > and that is wrong indication. > Sure ;-> > > >> +}; >> >> #endif /* _UAPI__LINUX_NETLINK_H */ >> diff --git a/lib/nlattr.c b/lib/nlattr.c >> index fb52435..c8aad7e 100644 >> --- a/lib/nlattr.c >> +++ b/lib/nlattr.c >> @@ -27,6 +27,20 @@ >> [NLA_S64] = sizeof(s64), >> }; >> >> +static int validate_nla_bitfield_32(const struct nlattr *nla, void *valid_data) > > This should be: > static int validate_nla_bitfield_32(const struct nlattr *nla, u32 *valid_flags_allowed) > > > Other than this 2 nits, this looks good. > > Are you sure Jiri? ;-> cheers, jamal
On 17-07-24 07:18 AM, Jiri Pirko wrote: > Mon, Jul 24, 2017 at 03:35:43AM CEST, jhs@mojatatu.com wrote: >> From: Jamal Hadi Salim <jhs@mojatatu.com> >> >> Generic bitflags attribute content sent to the kernel by user. >> With this type the user can either set or unset a flag in the >> kernel. >> >> The nla_value is a bitmap that defines the values being set >> The nla_selector is a bitmask that defines which value is legit. >> >> A check is made to ensure the rules that a kernel subsystem always >> conforms to bitflags the kernel already knows about. i.e >> if the user tries to set a bit flag that is not understood then >> the _it will be rejected_. >> >> In the most basic form, the user specifies the attribute policy as: >> [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = &myvalidflags }, >> >> where myvalidflags is the bit mask of the flags the kernel understands. >> >> If the user _does not_ provide myvalidflags then the attribute will >> also be rejected. >> >> Examples: >> nla_value = 0x0, and nla_selector = 0x1 >> implies we are selecting bit 1 and we want to set its value to 0. >> >> nla_value = 0x2, and nla_selector = 0x2 >> implies we are selecting bit 2 and we want to set its value to 1. > > Oh, 2 more things: > > [...] > ;-> ok will do that next opportunity (probably at 30K feet). cheers, jamal
On 7/23/17 7:35 PM, Jamal Hadi Salim wrote: > In the most basic form, the user specifies the attribute policy as: > [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = &myvalidflags }, > > where myvalidflags is the bit mask of the flags the kernel understands. > > If the user _does not_ provide myvalidflags then the attribute will > also be rejected. No other netlink attribute has this requirement. Users of the attributes are the only ones that know if a value is valid or not (e.g, attribute passing a device index) and those are always checked in line. Furthermore, you are locking this attribute into a static meaning of what is a valid value when flags can be valid or invalid based on other attributes passed in the request.
On 17-07-25 10:41 AM, David Ahern wrote: > On 7/23/17 7:35 PM, Jamal Hadi Salim wrote: >> In the most basic form, the user specifies the attribute policy as: >> [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = &myvalidflags }, >> >> where myvalidflags is the bit mask of the flags the kernel understands. >> >> If the user _does not_ provide myvalidflags then the attribute will >> also be rejected. > > No other netlink attribute has this requirement. This is the first one where we have to inspect content. We add things when we need them - as in this case. > Users of the attributes > are the only ones that know if a value is valid or not (e.g, attribute > passing a device index) and those are always checked in line. It doesnt make sense that every user of the API has to repeat that validation code. Same principle as someone specifying that a type is u32 and have the nla validation check it. At some point we never had the u32 validation code. Then it was factored out because everyone repeats the same boilerplate code. I see this in the same spirit. > Furthermore, you are locking this attribute into a static meaning of > what is a valid value when flags can be valid or invalid based on other > attributes passed in the request. > That doesnt disqualify that i factored out common code that all users of this nltype are going to cutnpaste. On the dependency on bit presence topic: I had added an "extra validation" ops - but it was distracting enough that i removed that patch altogether. cheers, jamal
Fri, Jul 28, 2017 at 03:51:41PM CEST, jhs@mojatatu.com wrote: >On 17-07-25 10:41 AM, David Ahern wrote: >> On 7/23/17 7:35 PM, Jamal Hadi Salim wrote: >> > In the most basic form, the user specifies the attribute policy as: >> > [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = &myvalidflags }, >> > >> > where myvalidflags is the bit mask of the flags the kernel understands. >> > >> > If the user _does not_ provide myvalidflags then the attribute will >> > also be rejected. >> >> No other netlink attribute has this requirement. > >This is the first one where we have to inspect content. We add things >when we need them - as in this case. > >> Users of the attributes >> are the only ones that know if a value is valid or not (e.g, attribute >> passing a device index) and those are always checked in line. > >It doesnt make sense that every user of the API has to repeat that >validation code. Same principle as someone specifying that a type is >u32 and have the nla validation check it. At some point we never had >the u32 validation code. Then it was factored out because everyone >repeats the same boilerplate code. >I see this in the same spirit. Agreed.
On 7/28/17 7:51 AM, Jamal Hadi Salim wrote: > On 17-07-25 10:41 AM, David Ahern wrote: >> On 7/23/17 7:35 PM, Jamal Hadi Salim wrote: >>> In the most basic form, the user specifies the attribute policy as: >>> [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = >>> &myvalidflags }, >>> >>> where myvalidflags is the bit mask of the flags the kernel understands. >>> >>> If the user _does not_ provide myvalidflags then the attribute will >>> also be rejected. >> >> No other netlink attribute has this requirement. > > This is the first one where we have to inspect content. We add things > when we need them - as in this case. Sure, the validation is required. My argument is that the validation should be done where other attributes are validated -- inline with its use. Nothing about this new bitfield says it must have a generic validation code. > >> Users of the attributes >> are the only ones that know if a value is valid or not (e.g, attribute >> passing a device index) and those are always checked in line. > > It doesnt make sense that every user of the API has to repeat that > validation code. Same principle as someone specifying that a type is > u32 and have the nla validation check it. At some point we never had > the u32 validation code. Then it was factored out because everyone > repeats the same boilerplate code. Every user of an attribute that uses a device index must verify the device index is valid. The same code is repeated over and over. Now you are suggesting to have 1 attribute whose content is validated by generic infra and the rest are validated inline by the code using it. I believe it is wrong and going to lead to problems.
Fri, Jul 28, 2017 at 04:19:06PM CEST, dsahern@gmail.com wrote: >On 7/28/17 7:51 AM, Jamal Hadi Salim wrote: >> On 17-07-25 10:41 AM, David Ahern wrote: >>> On 7/23/17 7:35 PM, Jamal Hadi Salim wrote: >>>> In the most basic form, the user specifies the attribute policy as: >>>> [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = >>>> &myvalidflags }, >>>> >>>> where myvalidflags is the bit mask of the flags the kernel understands. >>>> >>>> If the user _does not_ provide myvalidflags then the attribute will >>>> also be rejected. >>> >>> No other netlink attribute has this requirement. >> >> This is the first one where we have to inspect content. We add things >> when we need them - as in this case. > >Sure, the validation is required. My argument is that the validation >should be done where other attributes are validated -- inline with its >use. Nothing about this new bitfield says it must have a generic >validation code. > >> >>> Users of the attributes >>> are the only ones that know if a value is valid or not (e.g, attribute >>> passing a device index) and those are always checked in line. >> >> It doesnt make sense that every user of the API has to repeat that >> validation code. Same principle as someone specifying that a type is >> u32 and have the nla validation check it. At some point we never had >> the u32 validation code. Then it was factored out because everyone >> repeats the same boilerplate code. > >Every user of an attribute that uses a device index must verify the >device index is valid. The same code is repeated over and over. This is something different. You don't have NLA_IFINDEX. If you'd have it, might make sense to do validation on Netlink level. Ofc this is highly hypothetical. But in Jamal's case, there is indeed NLA_BITFIELD32 and this attribute type itself assumes some format. Therefore the validation on Netlink level makes sense here. At least that is how I feel it. > >Now you are suggesting to have 1 attribute whose content is validated by >generic infra and the rest are validated inline by the code using it. I >believe it is wrong and going to lead to problems.
On 17-07-28 10:19 AM, David Ahern wrote: > On 7/28/17 7:51 AM, Jamal Hadi Salim wrote: >> On 17-07-25 10:41 AM, David Ahern wrote: >>> On 7/23/17 7:35 PM, Jamal Hadi Salim wrote: >>>> In the most basic form, the user specifies the attribute policy as: >>>> [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = >>>> &myvalidflags }, >>>> >>>> where myvalidflags is the bit mask of the flags the kernel understands. >>>> >>>> If the user _does not_ provide myvalidflags then the attribute will >>>> also be rejected. >>> >>> No other netlink attribute has this requirement. >> >> This is the first one where we have to inspect content. We add things >> when we need them - as in this case. > > Sure, the validation is required. My argument is that the validation > should be done where other attributes are validated -- inline with its > use. Nothing about this new bitfield says it must have a generic > validation code. > Response below. >> >>> Users of the attributes >>> are the only ones that know if a value is valid or not (e.g, attribute >>> passing a device index) and those are always checked in line. >> >> It doesnt make sense that every user of the API has to repeat that >> validation code. Same principle as someone specifying that a type is >> u32 and have the nla validation check it. At some point we never had >> the u32 validation code. Then it was factored out because everyone >> repeats the same boilerplate code. > > Every user of an attribute that uses a device index must verify the > device index is valid. The same code is repeated over and over. > > Now you are suggesting to have 1 attribute whose content is validated by > generic infra and the rest are validated inline by the code using it. I > believe it is wrong and going to lead to problems. > Kernel side checking for device ifindex must know what a device ifindex means. That doesnt disqualify that the generic code checks that it is of the same size as a signed 32b, etc. That is generic stuff that can be factored out. In this case: Checking for whether bits selected are in the allowed range that the kernel understands, that the bit value are set in the right bit position, that the bits set in the correct bit value position are also selected in the transaction. That is generic code (which the content validation does). Checking what bit 5 means - that is specific per kernel user code. Checking that when bit 5 is selected it is only useful if bit 3 is unselected - that is specific to the kernel code consuming those bits. cheers, jamal
On 7/28/17 9:04 AM, Jamal Hadi Salim wrote: > > Kernel side checking for device ifindex must know what a device > ifindex means. > That doesnt disqualify that the generic code checks that it > is of the same size as a signed 32b, etc. That is generic > stuff that can be factored out. > > In this case: > Checking for whether bits selected are in the allowed range > that the kernel understands, that the bit value are set in > the right bit position, that the bits set in the correct bit > value position are also selected in the transaction. > That is generic code (which the content validation does). Create a helper function then. It's the validation of attribute content in 2 places that I object to. 1 attribute is validated in 1 place (generic infra for this bitfield attribute), the others are validated in line. Asymmetric validations is not a good design.
On 17-07-28 11:13 AM, David Ahern wrote: > On 7/28/17 9:04 AM, Jamal Hadi Salim wrote: >> >> Kernel side checking for device ifindex must know what a device >> ifindex means. >> That doesnt disqualify that the generic code checks that it >> is of the same size as a signed 32b, etc. That is generic >> stuff that can be factored out. >> >> In this case: >> Checking for whether bits selected are in the allowed range >> that the kernel understands, that the bit value are set in >> the right bit position, that the bits set in the correct bit >> value position are also selected in the transaction. >> That is generic code (which the content validation does). > > Create a helper function then. It's the validation of attribute content > in 2 places that I object to. 1 attribute is validated in 1 place > (generic infra for this bitfield attribute), the others are validated in > line. Asymmetric validations is not a good design. > What is not generic in what is being validated though? Content validation is when you say "This bit means the sky is blue". You have to know the meaning of the bit. The generic validation is opaque; "this bit is not supposed to be here". I would argue that "this is bit is not supposed to be here unless this other bit is present" is had to generalize but would be infra as well (why i provided the ops for it, but removed it so we could move on). cheers, jamal
diff --git a/include/net/netlink.h b/include/net/netlink.h index ef8e6c3..e33d1fb 100644 --- a/include/net/netlink.h +++ b/include/net/netlink.h @@ -178,6 +178,7 @@ enum { NLA_S16, NLA_S32, NLA_S64, + NLA_BITFIELD_32, __NLA_TYPE_MAX, }; @@ -206,6 +207,7 @@ enum { * NLA_MSECS Leaving the length field zero will verify the * given type fits, using it verifies minimum length * just like "All other" + * NLA_BITFIELD_32 A 32-bit bitmap/bitselector attribute * All other Minimum length of attribute payload * * Example: @@ -213,11 +215,13 @@ enum { * [ATTR_FOO] = { .type = NLA_U16 }, * [ATTR_BAR] = { .type = NLA_STRING, .len = BARSIZ }, * [ATTR_BAZ] = { .len = sizeof(struct mystruct) }, + * [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data = &myvalidflags }, * }; */ struct nla_policy { u16 type; u16 len; + void *validation_data; }; /** diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h index f86127a..0ac05a6 100644 --- a/include/uapi/linux/netlink.h +++ b/include/uapi/linux/netlink.h @@ -226,5 +226,22 @@ struct nlattr { #define NLA_ALIGN(len) (((len) + NLA_ALIGNTO - 1) & ~(NLA_ALIGNTO - 1)) #define NLA_HDRLEN ((int) NLA_ALIGN(sizeof(struct nlattr))) +/* Generic 32 bitflags attribute content sent to the kernel. + * + * The nla_value is a bitmap that defines the values being set + * The nla_selector is a bitmask that defines which value is legit + * + * Examples: + * nla_value = 0x0, and nla_selector = 0x1 + * implies we are selecting bit 1 and we want to set its value to 0. + * + * nla_value = 0x2, and nla_selector = 0x2 + * implies we are selecting bit 2 and we want to set its value to 1. + * + */ +struct nla_bitfield_32 { + __u32 nla_value; + __u32 nla_selector; +}; #endif /* _UAPI__LINUX_NETLINK_H */ diff --git a/lib/nlattr.c b/lib/nlattr.c index fb52435..c8aad7e 100644 --- a/lib/nlattr.c +++ b/lib/nlattr.c @@ -27,6 +27,20 @@ [NLA_S64] = sizeof(s64), }; +static int validate_nla_bitfield_32(const struct nlattr *nla, void *valid_data) +{ + const struct nla_bitfield_32 *nbf = nla_data(nla); + u32 *valid_flags_mask = valid_data; + + if (!valid_data) + return -EINVAL; + + if (nbf->nla_value & ~*valid_flags_mask) + return -EINVAL; + + return 0; +} + static int validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *policy) { @@ -46,6 +60,13 @@ static int validate_nla(const struct nlattr *nla, int maxtype, return -ERANGE; break; + case NLA_BITFIELD_32: + if (attrlen != sizeof(struct nla_bitfield_32)) + return -ERANGE; + + return validate_nla_bitfield_32(nla, pt->validation_data); + break; + case NLA_NUL_STRING: if (pt->len) minlen = min_t(int, attrlen, pt->len + 1);