netdev - Re: [PATCH 2/6] netlink: extend policy range validation

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] [day] [month] [year] [list]

Message-ID: <c64504ca3ec2946f4d1575b7a28279f606fbd3d9.camel@sipsolutions.net>
Date:   Tue, 30 Apr 2019 08:51:43 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, dsa@...ulusnetworks.com,
        pablo@...filter.org
Subject: Re: [PATCH 2/6] netlink: extend policy range validation

On Mon, 2019-04-29 at 22:49 -0400, David Miller wrote:
> From: Johannes Berg <johannes@...solutions.net>
> Date: Fri, 26 Apr 2019 14:13:02 +0200
> 
> >   *                         NLA_POLICY_RANGE() macros.
> > + *    NLA_U8,
> > + *    NLA_U16,
> > + *    NLA_U32,
> > + *    NLA_U64              If the validation_type field instead is set to
> > + *                         NLA_VALIDATE_RANGE_PTR, `range' must be a pointer
> > + *                         to a struct netlink_range_validation that indicates
> > + *                         the min/max values.
> > + *                         Use NLA_POLICY_FULL_RANGE().
> > + *    NLA_S8,
> > + *    NLA_S16,
> > + *    NLA_S32,
> > + *    NLA_S64              If the validation_type field instead is set to
> > + *                         NLA_VALIDATE_RANGE_PTR, `range_signed' must be a
> > + *                         pointer to a struct netlink_range_validation_signed
> > + *                         that indicates the min/max values.
> > + *                         Use NLA_POLICY_FULL_RANGE_SIGNED().
> 
> Documentation and datastructure says that "range_signed" member should be set
> for signed ranges, however:
> 
> > +#define NLA_POLICY_FULL_RANGE(tp, _range) {          \
> > +     .type = NLA_ENSURE_UINT_TYPE(tp),               \
> > +     .validation_type = NLA_VALIDATE_RANGE_PTR,      \
> > +     .range = _range,                                \
> > +}
> > +
> > +#define NLA_POLICY_FULL_RANGE_SIGNED(tp, _range) {   \
> > +     .type = NLA_ENSURE_SINT_TYPE(tp),               \
> > +     .validation_type = NLA_VALIDATE_RANGE_PTR,      \
> > +     .range = _range,                                \
> > +}
> 
> The NLA_POLICY_FULL_RANGE_SIGNED macros sets 'range' not 'range_signed'.

D'oh. Copy/paste error, and I must've missed the compiler warning that
should appear here on usage then. At least I'm pretty sure I tested that
with the policy exposition patch.

Will fix.

> Also, since range and range_signed are in a union however there is only one
> NLA_VALIDATE_RANGE_PTR type, how does one differentiate between signed and
> unsigned ranges exactly?

Based on the type - NLA_S* or NLA_U*. See the NLA_ENSURE_SINT_TYPE() and
NLA_ENSURE_UINT_TYPE() in the macros - that ensures you can only use
NLA_POLICY_FULL_RANGE_SIGNED() with NLA_S*, and NLA_POLICY_FULL_RANGE()
with NLA_U*.

johannes