Message ID | 1493562519-15563-1-git-send-email-jhs@emojatatu.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
From: Jamal Hadi Salim <jhs@mojatatu.com> Date: Sun, 30 Apr 2017 10:28:39 -0400 > 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. You asked for feedback, here it is :-) I think this is overengineered. Just define a u32 for the value, and mask which defines which bits are legitimate and defined. Any bit outside of the legitimate mask must be zero. Simple.
On 17-05-02 03:03 PM, David Miller wrote: > From: Jamal Hadi Salim <jhs@mojatatu.com> > Date: Sun, 30 Apr 2017 10:28:39 -0400 > >> 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. > > You asked for feedback, here it is :-) > > I think this is overengineered. > > Just define a u32 for the value, and mask which defines which bits are > legitimate and defined. Any bit outside of the legitimate mask must > be zero. > That is what it does but as a nla type. It has a validator ops() in case someone wants to override the default validator. Is the ops the over-engineering you refer to or merely making it an nla type is a problem? cheers, jamal
Ping.. Following up On 17-05-02 09:52 PM, Jamal Hadi Salim wrote: > On 17-05-02 03:03 PM, David Miller wrote: >> From: Jamal Hadi Salim <jhs@mojatatu.com> >> Date: Sun, 30 Apr 2017 10:28:39 -0400 >> >>> 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. >> >> You asked for feedback, here it is :-) >> >> I think this is overengineered. >> >> Just define a u32 for the value, and mask which defines which bits are >> legitimate and defined. Any bit outside of the legitimate mask must >> be zero. >> > > That is what it does but as a nla type. It has a validator ops() in > case someone wants to override the default validator. > Is the ops the over-engineering you refer to or merely making it an > nla type is a problem? > Since its been a month, a reminder of what the patch introduced a new netlink type, NLA_FLAG_BITS UAPI structure: struct nla_bit_flags { u32 nla_flag_values; u32 nla_flag_selector; } User to kernel example: --- nla_flag_values = 0x0, and nla_flag_selector = 0x1 implies we are selecting bit 1 and we want to set its value to 0. nla_flag_values = 0x2, and nla_flag_selector = 0x2 implies we are selecting bit 2 and we want to set its value to 1. --- A kernel subsystem specifies validation rules of the following nature: ---- [ATTR_GOO] = { .type = NLA_FLAG_BITS, .validation_data = &myvalidflags }, where myvalidflags is the bit mask of the flags the kernel understands. We reject any bitmask of values that dont fit myvalidflags. The user can also specify their own validation callback as so: [ATTR_GOO] = { .type = MYTYPE, .validation_data = &myvalidation_data, .validate_content = mycontent_validator }, myvalidation_data is the allowed bitmap as before With validator callback looking like: int mycontent_validator(const struct nlattr *nla, void *valid_data) { const struct myattribute *user_data = nla_data(nla); struct myvalidation_struct *valid_data_constraint = valid_data; ... validate user_data against valid_data_constraint ... ... return appropriate error code etc ... } ------- So Dave ;-> Are you suggesting I get rid of the validation op? cheers, jamal
diff --git a/include/net/netlink.h b/include/net/netlink.h index 0170917..8ab9784 100644 --- a/include/net/netlink.h +++ b/include/net/netlink.h @@ -6,6 +6,11 @@ #include <linux/jiffies.h> #include <linux/in6.h> +struct nla_bit_flags { + u32 nla_flag_values; + u32 nla_flag_selector; +}; + /* ======================================================================== * Netlink Messages and Attributes Interface (As Seen On TV) * ------------------------------------------------------------------------ @@ -178,6 +183,7 @@ enum { NLA_S16, NLA_S32, NLA_S64, + NLA_FLAG_BITS, __NLA_TYPE_MAX, }; @@ -206,6 +212,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_FLAG_BITS A bitmap/bitselector attribute * All other Minimum length of attribute payload * * Example: @@ -213,11 +220,15 @@ enum { * [ATTR_FOO] = { .type = NLA_U16 }, * [ATTR_BAR] = { .type = NLA_STRING, .len = BARSIZ }, * [ATTR_BAZ] = { .len = sizeof(struct mystruct) }, + * [ATTR_GOO] = { .type = NLA_FLAG_BITS, .validation_data = &myvalidflags }, * }; */ struct nla_policy { u16 type; u16 len; + void *validation_data; + int (*validate_content)(const struct nlattr *nla, + const void *validation_data); }; /** diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h index cce0613..3691d8d 100644 --- a/include/uapi/linux/rtnetlink.h +++ b/include/uapi/linux/rtnetlink.h @@ -179,6 +179,23 @@ struct rtattr { #define RTA_DATA(rta) ((void*)(((char*)(rta)) + RTA_LENGTH(0))) #define RTA_PAYLOAD(rta) ((int)((rta)->rta_len) - RTA_LENGTH(0)) +/* Generic bitflags attribute content sent to the kernel. + * + * The nla_flag_values is a bitmap that defines the values being set + * The nla_flag_selector is a bitmask that defines which value is legit + * + * Examples: + * nla_flag_values = 0x0, and nla_flag_selector = 0x1 + * implies we are selecting bit 1 and we want to set its value to 0. + * + * nla_flag_values = 0x2, and nla_flag_selector = 0x2 + * implies we are selecting bit 2 and we want to set its value to 1. + * + */ +struct __nla_bit_flags { + __u32 nla_flag_values; + __u32 nla_flag_selector; +}; diff --git a/lib/nlattr.c b/lib/nlattr.c index a7e0b16..78fed43 100644 --- a/lib/nlattr.c +++ b/lib/nlattr.c @@ -27,6 +27,21 @@ [NLA_S64] = sizeof(s64), }; +static int validate_nla_bit_flags(const struct nlattr *nla, void *valid_data) +{ + const struct nla_bit_flags *nbf = nla_data(nla); + u32 *valid_flags_mask = valid_data; + + if (!valid_data) + return -EINVAL; + + + if (nbf->nla_flag_values & ~*valid_flags_mask) + return -EINVAL; + + return 0; +} + static int validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *policy) { @@ -46,6 +61,13 @@ static int validate_nla(const struct nlattr *nla, int maxtype, return -ERANGE; break; + case NLA_FLAG_BITS: + if (attrlen != 8) /* 2 x 32 bits */ + return -ERANGE; + + return validate_nla_bit_flags(nla, pt->validation_data); + break; + case NLA_NUL_STRING: if (pt->len) minlen = min_t(int, attrlen, pt->len + 1); @@ -103,6 +125,9 @@ static int validate_nla(const struct nlattr *nla, int maxtype, return -ERANGE; } + if (pt->validate_content) + return pt->validate_content(nla, pt->validation_data); + return 0; }