| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CALx6S35OeP4nR+AB2Pujamcork3WVz3KyZi4qSGKfL5CXkMj6w@mail.gmail.com>
Date: Sat, 14 Dec 2019 11:25:31 -0800
From: Tom Herbert <tom@...bertland.com>
To: Simon Horman <simon.horman@...ronome.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Linux Kernel Network Developers <netdev@...r.kernel.org>,
Tom Herbert <tom@...ntonium.net>
Subject: Re: [PATCH v5 net-next 6/7] ip6tlvs: Add netlink interface
On Mon, Oct 7, 2019 at 5:39 AM Simon Horman <simon.horman@...ronome.com> wrote:
>
> On Thu, Oct 03, 2019 at 02:58:03PM -0700, Tom Herbert wrote:
> > From: Tom Herbert <tom@...ntonium.net>
> >
> > Add a netlink interface to manage the TX TLV parameters. Managed
> > parameters include those for validating and sending TLVs being sent
> > such as alignment, TLV ordering, length limits, etc.
> >
> > Signed-off-by: Tom Herbert <tom@...bertland.com>
>
> Hi Tom,
>
> I am wondering if you considered including extack support in this code.
>
Good point, will look into adding it.
> ...
>
> > diff --git a/include/uapi/linux/ipeh.h b/include/uapi/linux/ipeh.h
> > index dbf0728..bac36a7 100644
> > --- a/include/uapi/linux/ipeh.h
> > +++ b/include/uapi/linux/ipeh.h
> > @@ -21,4 +21,33 @@ enum {
> > IPEH_TLV_PERM_MAX = IPEH_TLV_PERM_NO_CHECK
> > };
> >
> > +/* NETLINK_GENERIC related info for IP TLVs */
> > +
> > +enum {
> > + IPEH_TLV_ATTR_UNSPEC,
> > + IPEH_TLV_ATTR_TYPE, /* u8, > 1 */
> > + IPEH_TLV_ATTR_ORDER, /* u16 */
> > + IPEH_TLV_ATTR_ADMIN_PERM, /* u8, perm value */
> > + IPEH_TLV_ATTR_USER_PERM, /* u8, perm value */
>
> My reading of struct tlv_tx_params is that admin_perm and user_perm are
> 2-bit entities whose valid values are currently 0, 1 and 2. Perhaps that
> would be worth noting here in keeping with restrictions noted for other
> attributes.
>
Okay.
> > + IPEH_TLV_ATTR_CLASS, /* u8, 3 bit flags */
> > + IPEH_TLV_ATTR_ALIGN_MULT, /* u8, 1 to 16 */
> > + IPEH_TLV_ATTR_ALIGN_OFF, /* u8, 0 to 15 */
> > + IPEH_TLV_ATTR_MIN_DATA_LEN, /* u8 (option data length) */
> > + IPEH_TLV_ATTR_MAX_DATA_LEN, /* u8 (option data length) */
> > + IPEH_TLV_ATTR_DATA_LEN_MULT, /* u8, 1 to 16 */
> > + IPEH_TLV_ATTR_DATA_LEN_OFF, /* u8, 0 to 15 */
> > +
> > + __IPEH_TLV_ATTR_MAX,
> > +};
> > +
> > +#define IPEH_TLV_ATTR_MAX (__IPEH_TLV_ATTR_MAX - 1)
>
> ...
>
> > diff --git a/net/ipv6/exthdrs_common.c b/net/ipv6/exthdrs_common.c
> > index feaa4a6..e3c1f33 100644
> > --- a/net/ipv6/exthdrs_common.c
> > +++ b/net/ipv6/exthdrs_common.c
> > @@ -454,6 +454,244 @@ int __ipeh_tlv_unset(struct tlv_param_table *tlv_param_table,
>
> ...
>
> > +int ipeh_tlv_nl_cmd_set(struct tlv_param_table *tlv_param_table,
> > + struct genl_family *tlv_nl_family,
> > + struct sk_buff *skb, struct genl_info *info)
> > +{
> > + struct tlv_params new_params;
> > + struct tlv_proc *tproc;
> > + unsigned char type;
> > + unsigned int v;
> > + int retv = -EINVAL;
> > +
> > + if (!info->attrs[IPEH_TLV_ATTR_TYPE])
> > + return -EINVAL;
> > +
> > + type = nla_get_u8(info->attrs[IPEH_TLV_ATTR_TYPE]);
> > + if (type < 2)
> > + return -EINVAL;
> > +
> > + rcu_read_lock();
> > +
> > + /* Base new parameters on existing ones */
> > + tproc = ipeh_tlv_get_proc_by_type(tlv_param_table, type);
> > + new_params = tproc->params;
> > +
> > + if (info->attrs[IPEH_TLV_ATTR_ORDER]) {
> > + v = nla_get_u16(info->attrs[IPEH_TLV_ATTR_ORDER]);
> > + new_params.t.preferred_order = v;
> > + }
> > +
> > + if (info->attrs[IPEH_TLV_ATTR_ADMIN_PERM]) {
> > + v = nla_get_u8(info->attrs[IPEH_TLV_ATTR_ADMIN_PERM]);
> > + if (v > IPEH_TLV_PERM_MAX)
> > + goto out;
> > + new_params.t.admin_perm = v;
> > + }
> > +
> > + if (info->attrs[IPEH_TLV_ATTR_USER_PERM]) {
> > + v = nla_get_u8(info->attrs[IPEH_TLV_ATTR_USER_PERM]);
> > + if (v > IPEH_TLV_PERM_MAX)
> > + goto out;
> > + new_params.t.user_perm = v;
> > + }
> > +
> > + if (info->attrs[IPEH_TLV_ATTR_CLASS]) {
> > + v = nla_get_u8(info->attrs[IPEH_TLV_ATTR_CLASS]);
> > + if (!v || (v & ~IPEH_TLV_CLASS_FLAG_MASK))
> > + goto out;
> > + new_params.t.class = v;
> > + }
> > +
> > + if (info->attrs[IPEH_TLV_ATTR_ALIGN_MULT]) {
> > + v = nla_get_u8(info->attrs[IPEH_TLV_ATTR_ALIGN_MULT]);
> > + if (v > 16 || v < 1)
> > + goto out;
> > + new_params.t.align_mult = v - 1;
> > + }
> > +
> > + if (info->attrs[IPEH_TLV_ATTR_ALIGN_OFF]) {
> > + v = nla_get_u8(info->attrs[IPEH_TLV_ATTR_ALIGN_OFF]);
> > + if (v > 15)
> > + goto out;
> > + new_params.t.align_off = v;
> > + }
> > +
> > + if (info->attrs[IPEH_TLV_ATTR_MAX_DATA_LEN])
> > + new_params.t.max_data_len =
> > + nla_get_u8(info->attrs[IPEH_TLV_ATTR_MAX_DATA_LEN]);
> > +
> > + if (info->attrs[IPEH_TLV_ATTR_MIN_DATA_LEN])
> > + new_params.t.min_data_len =
> > + nla_get_u8(info->attrs[IPEH_TLV_ATTR_MIN_DATA_LEN]);
> > +
> > + if (info->attrs[IPEH_TLV_ATTR_DATA_LEN_MULT]) {
> > + v = nla_get_u8(info->attrs[IPEH_TLV_ATTR_DATA_LEN_MULT]);
> > + if (v > 16 || v < 1)
> > + goto out;
> > + new_params.t.data_len_mult = v - 1;
> > + }
>
> Is some sanity checking warranted for the min/max data len values.
> f.e. that min <= max ?
Okay, will add.
>
> > +
> > + if (info->attrs[IPEH_TLV_ATTR_DATA_LEN_OFF]) {
> > + v = nla_get_u8(info->attrs[IPEH_TLV_ATTR_DATA_LEN_OFF]);
> > + if (v > 15)
> > + goto out;
> > + new_params.t.data_len_off = v;
> > + }
> > +
> > + retv = ipeh_tlv_set_params(tlv_param_table, type, &new_params);
> > +
> > +out:
> > + rcu_read_unlock();
> > + return retv;
> > +}
> > +EXPORT_SYMBOL(ipeh_tlv_nl_cmd_set);
>
> ...
Powered by blists - more mailing lists