linux-kernel - Re: [PATCH] atm: he: fix potential ioremap leak of membase in he

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: <c379d193-5408-a514-9f37-eb93585557ab@hust.edu.cn>
Date:   Fri, 10 Mar 2023 21:15:43 +0800
From:   Dongliang Mu <dzm91@...t.edu.cn>
To:     Francois Romieu <romieu@...zoreil.com>,
        Gencen Gan <u202011061@...il.com>
Cc:     Chas Williams <3chas3@...il.com>,
        linux-atm-general@...ts.sourceforge.net, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Dan Carpenter <error27@...il.com>
Subject: Re: [PATCH] atm: he: fix potential ioremap leak of membase in he_dev



On 3/10/23 19:28, Francois Romieu wrote:
> Gencen Gan <u202011061@...il.com> :
>> In the function he_start() in drivers/atm/he.c, there
>> is no unmapping of he_dev->membase in the branch that
>> exits due to an error like reset failure, which may
>> cause a memory leak.
> 
> Why would he_dev->membase not be unmapped in he_stop() ?
> 
> he_stop() is paired with he_start() as soon a he_start() returns
> anything different from 0 in he_init_one(). I see no other place
> where he_start() is used.

Yes, you're right. We will check more about reports from the static 
checker Smatch.

Smatch should make a false positive here, I think it might be that, 
Smatch has an assumption about do and its paired undo functions. The do 
function should clean up its own allocation operations. And the paired 
undo function can be only called if the do function succeeds.

+cc Dan Carpenter

Maybe @Dan could tell more about this point.

> 
> The atm_dev/he_dev pointers also seem correctly set.
>