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

From: Bodo Stroesser
Date: Tue Jul 27 2021 - 03:27:55 EST

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.
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.