linux-kernel - Re: [PATCH] scsi: target: Fix data corruption under concurrent target configuration

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: <20230520084600.GC20571@yadro.com>
Date:   Sat, 20 May 2023 11:46:00 +0300
From:   Dmitry Bogdanov <d.bogdanov@...ro.com>
To:     Grzegorz Uriasz <gorbak25@...il.com>
CC:     "Martin K. Petersen" <martin.petersen@...cle.com>,
        <linux-scsi@...r.kernel.org>, <target-devel@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <dutkahugo@...il.com>
Subject: Re: [PATCH] scsi: target: Fix data corruption under concurrent
 target configuration

Hi Grzegorz,

On Sat, May 20, 2023 at 02:26:14AM +0200, Grzegorz Uriasz wrote:
> 
> This fixes data corruptions arising from concurrent enabling of a target
> devices. When multiple enable calls are made concurrently then it is
> possible for the target device to be set up twice which results in a
> kernel BUG.
> Introduces a per target device mutex for serializing enable requests.

Device enable call is already secured by configfs per-file mutex. That
is actually per device. So Enable procedures are already not executed
simulteniously.

Look like you wrongly identified the root cause of double list_add.

If you have an evidence that dev->dev_flags could have no DF_CONFIGURED
bit, then it meeans that it (dev_flags) is raced in other
configuration actions (udev_path, vpd_unit_serial, alias).
Bits in dev->dev_flags are written not atomically and if you writes to
enable, alias, udev_path,unit_serial files simulteniously, then some
bits could be lost.

IHMO the best solution is to make dev_flags changes be atomical.

BR,
 Dmitry