netdev - Re: [PATCH RFC net-next 3/5] bonding: add ip6_addr for bond_opt

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]

Date:   Thu, 27 Jan 2022 20:09:36 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     Nikolay Aleksandrov <nikolay@...dia.com>
Cc:     netdev@...r.kernel.org, Jay Vosburgh <j.vosburgh@...il.com>,
        Veaceslav Falico <vfalico@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        David Ahern <dsahern@...il.com>
Subject: Re: [PATCH RFC net-next 3/5] bonding: add ip6_addr for bond_opt_value

On Thu, Jan 27, 2022 at 10:56:29AM +0200, Nikolay Aleksandrov wrote:
> You're right in that we shouldn't overload value. My point was that bond_opt_val is supposed to be generic,
> also it wouldn't work as expected for bond_opt_parse(). Perhaps a better solution would be to add a generic
> extra storage field and length and initialize them with a helper that copies the needed bytes there. As for

Not sure if I understand your suggestion correctly. Do you mean add a field
in bond_opt_value like:

#define	MAX_LEN	128

struct bond_opt_value {
        char *string;
        u64 value;
        u32 flags;
        char extra[MAX_LEN];
};

And init it before using?

or define a char *extra and alloc/init the memory when using it?

Thanks
Hangbin

> value in that case you can just set it to 0, since all of this would be used internally the options which
> support this new extra storage would expect it and should error out if it's missing (wrong/zero length).
> Maybe something like:
> static inline void __bond_opt_init(struct bond_opt_value *optval,
> -				   char *string, u64 value)
> +				   char *string, u64 value,
> +				   void *extra, size_t extra_len)
> 
> with sanity and length checks of course, and:
> +#define bond_opt_initextra(optval, extra, len) __bond_opt_init(optval, NULL, 0, extra, len)
> 
> It is similar to your solution, but it can be used by other options to store larger values and
> it uses the value field as indicator that string shouldn't be parsed.
> 
> There are other alternatives like using the bond_opt_val flags to denote what has been set instead
> of using the current struct field checks, but they would cause much more changes that seems
> unnecessary just for this case.
> 
> Cheers,
>  Nik
> 
> 
> 
>