linux-kernel - Re: [PATCH 2/4] configfs: Fix writing at a non-zero offset

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: <fec30933-46b1-1085-1af1-1fd0d2265981@gmail.com>
Date:   Tue, 27 Jul 2021 09:27:48 +0200
From:   Bodo Stroesser <bostroesser@...il.com>
To:     Bart Van Assche <bvanassche@....org>,
        Christoph Hellwig <hch@....de>
Cc:     Joel Becker <jlbec@...lplan.org>, linux-kernel@...r.kernel.org,
        "Martin K . Petersen" <martin.petersen@...cle.com>,
        Yanko Kaneti <yaneti@...lera.com>,
        Brendan Higgins <brendanhiggins@...gle.com>
Subject: Re: [PATCH 2/4] configfs: Fix writing at a non-zero offset

On 27.07.21 05:17, Bart Van Assche wrote:
> On 7/26/21 5:54 PM, Bodo Stroesser wrote:
>> The new behavior can also cause trouble with existing store handlers.
>> Example:
>> The tcmu attribute files cmd_time_out and qfull_time_out just take a
>> string containing the decimal formatted number of seconds of the
>> timeout. Each number up to now had to be transferred in a single write.
>> Assume the old value is 30 and we want to change to 19. If userspace
>> writes byte by byte, you end up calling
>> store(item, "1\0", 1) and then
>> store(item, "19\9", 2).
>> If these quick changes do not cause trouble in tcmu's scsi cmd handling,
>> then think what happens, if userspace is interrupted between the two
>> writes. Allowing to split the writes cause a loss of "atomicity".
> 
>  From Documentation/filesystems/configfs.rst, for normal attributes:
> "Configfs expects write(2) to store the entire buffer at once." In other
> words, the behavior for partial writes is undocumented. My changes
> preserve the behavior if a buffer is written in its entirety. I do not
> agree that my changes can cause trouble for existing store handlers.
> 

I agree. I was not precise.

What I meant is, that changing the source code in such a way, that
writing a buffer in multiple writes works in general, could cause
trouble in case userspace uses this.

But for special syscall sequences your changes still change the result
on existing configfs files. Example:

1) userspace program opens qfull_time_out
2) userspace program writes "90", count=2 to set timeout to 90 sec
3) userspace again wants to change timeout, so it writes "55", count=2

Before the changes we end up with timeout being 55 seconds. After the
change - due to data gathering - we finally have timeout 9055 seconds.

BR,
Bodo