netdev - Re: [PATCH net-next v4 3/3] Validate all netlink attributes and return error if any of the validation fails.

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CA+CtxLRR1LiotUPk=Pi995+6PF8ys72Zx1xbAevX7CTF9H4u9g@mail.gmail.com>
Date:	Tue, 17 Mar 2015 10:29:28 +0530
From:	Siva Mannem <siva.mannem.lnx@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	Netdev <netdev@...r.kernel.org>, Scott Feldman <sfeldma@...il.com>
Subject: Re: [PATCH net-next v4 3/3] Validate all netlink attributes and
 return error if any of the validation fails.

On Tue, Mar 17, 2015 at 9:54 AM, David Miller <davem@...emloft.net> wrote:
> From: Siva Mannem <siva.mannem.lnx@...il.com>
> Date: Tue, 17 Mar 2015 04:32:44 +0530
>
>> This patch validates all netlink attributes and return error if any of the
>> validation fails.
>>
>> Signed-off-by: Siva Mannem <siva.mannem.lnx@...il.com>
>> Suggested-by: David Miller <davem@...emloft.net>
>
> You have to remove the range checking code from all the other
> functions too.
>
> Then no errors can be returned by them at all actually and that's an
> really nice cleanup.
>
> Please don't make microscopic narrow changes like this, think about
> the higher level implications and the things that are no longer
> necessary as a result.
>
> Thanks.

These functions(br_set_forward_delay/hello_time/max_age/ageing_time)
are also called from other places i.e old_dev_ioctl() and sysfs code. If
range checking has to be removed from these functions, then the same
has to be added in the path from where they are being invoked.

As Scott mentioned,  these functions can be enhanced to take
extra argument to avoid extra range checking happening in br_changelink()
path. Is this approach ok?
-- 
Regards,
Siva Mannem.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html