netdev - Re: [PATCH net-next 2/2] openvswitch: Add timeout support to ct action

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAG1aQh+kdpaSb9Dh8jkMJNP79CuHx2oTMsP4yFAgo=ER0ODQAg@mail.gmail.com>
Date:   Thu, 21 Mar 2019 16:05:19 -0700
From:   Yi-Hung Wei <yihung.wei@...il.com>
To:     Pravin Shelar <pshelar@....org>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/2] openvswitch: Add timeout support to ct action

> > +static void ovs_ct_add_timeout(struct net *net, struct nf_conn *ct,
> > +                              const char *timeout_name, u16 l3num, u8 l4num)
> > +{
> This code looks very similar to xt_ct_set_timeout(), can you refactor
> it to avoid code duplication?

Thanks Prvain's feedback. I will export the set timeout function and
use that to avoid duplication.

> >  static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
> > @@ -1550,6 +1607,15 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
> >                         info->have_eventmask = true;
> >                         info->eventmask = nla_get_u32(a);
> >                         break;
> > +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
> > +               case OVS_CT_ATTR_TIMEOUT:
> > +                       memcpy(info->timeout, nla_data(a), nla_len(a));
> Before copying timeout, we need to check sizeof source string.
> 'nla_len(a)' needs to be less than CTNL_TIMEOUT_NAME_MAX.

There are some exiting checking in parse_ct() as below. That should
check the sizeof source string already.
....
        nla_for_each_nested(a, attr, rem) {
                int type = nla_type(a);
                int maxlen;
                int minlen;
......
                maxlen = ovs_ct_attr_lens[type].maxlen;
                minlen = ovs_ct_attr_lens[type].minlen;
                if (nla_len(a) < minlen || nla_len(a) > maxlen) {
                        OVS_NLERR(log,
                                  "Conntrack attr type has unexpected
length (type=%d, length=%d, expected=%d)",
                                  type, nla_len(a), maxlen);
                        return -EINVAL;
                }

I will respin v2 soon.

Thanks,

-Yi-Hung