netdev - Re: [PATCH 0/3] fix a couple of atm->phy

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: <CAKgT0UftdTobwgA6hi=CdOfQ+1fdozhPs89fDmapbvcp7jLASw@mail.gmail.com>
Date:   Mon, 8 Mar 2021 09:47:04 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Tong Zhang <ztong0001@...il.com>
Cc:     Chas Williams <3chas3@...il.com>,
        linux-atm-general@...ts.sourceforge.net,
        Netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/3] fix a couple of atm->phy_data related issues

On Mon, Mar 8, 2021 at 12:39 AM Tong Zhang <ztong0001@...il.com> wrote:
>
> there are two drivers(zatm and idt77252) using PRIV() (i.e. atm->phy_data)
> to store private data, but the driver happens to populate wrong
> pointers: atm->dev_data. which actually cause null-ptr-dereference in
> following PRIV(dev). This patch series attemps to fix those two issues
> along with a typo in atm struct.
>
> Tong Zhang (3):
>   atm: fix a typo in the struct description
>   atm: uPD98402: fix incorrect allocation
>   atm: idt77252: fix null-ptr-dereference
>
>  drivers/atm/idt77105.c | 4 ++--
>  drivers/atm/uPD98402.c | 2 +-
>  include/linux/atmdev.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)

For the 2 phys you actually seen null pointer dereferences or are your
changes based on just code review?

I ask because it seems like this code has been this way since 2005 and
in the case of uPD98402_start the code doesn't seem like it should
function the way it was as PRIV is phy_data and there being issues
seems pretty obvious since the initialization of things happens
immediately after the allocation.

I'm just wondering if it might make more sense to drop the code if it
hasn't been run in 15+ years rather than updating it?