linux-kernel - Re: [PATCH] usb: gadget: f_fs: Fix setting of device and driver data cross-references

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: <20210604110503.GA23002@vmlxhi-102.adit-jv.com>
Date:   Fri, 4 Jun 2021 13:05:03 +0200
From:   Eugeniu Rosca <erosca@...adit-jv.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Felipe Balbi <balbi@...nel.org>,
        Andrew Gabbasov <andrew_gabbasov@...tor.com>
CC:     <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Eugeniu Rosca <erosca@...adit-jv.com>,
        Eugeniu Rosca <roscaeugeniu@...il.com>
Subject: Re: [PATCH] usb: gadget: f_fs: Fix setting of device and driver data
 cross-references

Hello,

On Thu, Jun 03, 2021 at 12:15:07PM -0500, Andrew Gabbasov wrote:
> FunctionFS device structure 'struct ffs_dev' and driver data structure
> 'struct ffs_data' are bound to each other with cross-reference pointers
> 'ffs_data->private_data' and 'ffs_dev->ffs_data'. While the first one
> is supposed to be valid through the whole life of 'struct ffs_data'
> (and while 'struct ffs_dev' exists non-freed), the second one is cleared
> in 'ffs_closed()' (called from 'ffs_data_reset()' or the last
> 'ffs_data_put()'). This can be called several times, alternating in
> different order with 'ffs_free_inst()', that, if possible, clears
> the other cross-reference.
> 
> As a result, different cases of these calls order may leave stale
> cross-reference pointers, used when the pointed structure is already
> freed. Even if it occasionally doesn't cause kernel crash, this error
> is reported by KASAN-enabled kernel configuration.
> 
> For example, the case [last 'ffs_data_put()' - 'ffs_free_inst()'] was
> fixed by commit cdafb6d8b8da ("usb: gadget: f_fs: Fix use-after-free in
> ffs_free_inst").
> 
> The other case ['ffs_data_reset()' - 'ffs_free_inst()' - 'ffs_data_put()']
> now causes KASAN reported error [1], when 'ffs_data_reset()' clears
> 'ffs_dev->ffs_data', then 'ffs_free_inst()' frees the 'struct ffs_dev',
> but can't clear 'ffs_data->private_data', which is then accessed
> in 'ffs_closed()' called from 'ffs_data_put()'. This happens since
> 'ffs_dev->ffs_data' reference is cleared too early.
> 
> Moreover, one more use case, when 'ffs_free_inst()' is called immediately
> after mounting FunctionFS device (that is before the descriptors are
> written and 'ffs_ready()' is called), and then 'ffs_data_reset()'
> or 'ffs_data_put()' is called from accessing "ep0" file or unmounting
> the device. This causes KASAN error report like [2], since
> 'ffs_dev->ffs_data' is not yet set when 'ffs_free_inst()' can't properly
> clear 'ffs_data->private_data', that is later accessed to freed structure.

I confirm there are at least two KASAN use-after-free issues
consistently/100% reproducible on v5.13-rc4-88-gf88cd3fb9df2:

https://gist.github.com/erosca/b5976a96789e574b319cb9e076938b5c
https://gist.github.com/erosca/4ded55ed32f0133bc2f4ccfe821c7776

These two can no longer be seen after the patch is applied.

In addition, below static analysis tools did not spot any regressions:
cppcheck 2.4, smatch v0.5.0-7445-g58776ae33ae8, make W=1, coccicheck

Reviewed-by: Eugeniu Rosca <erosca@...adit-jv.com>
Tested-by: Eugeniu Rosca <erosca@...adit-jv.com>

-- 
Best regards,
Eugeniu Rosca