netdev - Re: [PATCH iproute2] libnetlink.c, ss.c: properly handle fread() error

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: <20191028212128.1b8c5054@hermes.lan>
Date:   Mon, 28 Oct 2019 21:21:28 -0700
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Michał Łyszczek <michal.lyszczek@...c.pl>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] libnetlink.c, ss.c: properly handle fread()
 error

On Thu, 24 Oct 2019 23:20:01 +0200
Michał Łyszczek <michal.lyszczek@...c.pl> wrote:

> fread(3) returns size_t data type which is unsigned, thus check
> `if (fread(...) < 0)' is always false. To check if fread(3) has
> failed, user should check if return is 0 and then check error
> indicator with ferror(3).
> 
> Signed-off-by: Michał Łyszczek <michal.lyszczek@...c.pl>

You did find something that probably has been broken for a long time.

First off, not sure why libnetlink is using fread here anyway.
It adds another copy to all I/O which can matter with 1M routes.

Also the man page for fread() implies that truncated reads (not
just zero) can happen on error. Better to check that full read was
completed or at least a valid netlink header?