casaroli opened a new pull request, #21466:
URL: https://github.com/apache/nuttx/pull/21466
## Summary
A write past EOF on a FAT file zeroes the last cluster of the file when the
file size is an exact multiple of the cluster size. The data in that cluster is
lost, on the disk too. The code is in `fat_get_sectors()` since 479c3932914.
`fat_get_sectors()` zeroes the gap in the last cluster from `s_size &
(clu_size - 1)`. This value is 0 at a cluster boundary, but that cluster is
full of file data and has no gap. This change skips area (1) at a cluster
boundary. The gap then starts in the next cluster, and areas (2) and (3) zero
it. A write at EOF from a cluster boundary already takes this path.
```c
fd = open("/fat/A.BIN", O_RDWR); /* The file size is one cluster */
lseek(fd, clu_size + 100, SEEK_SET);
write(fd, buf, 10);
close(fd); /* Bytes 0 to clu_size - 1 are now 0 */
```
## Impact
Bug fix for all FAT types, build modes and architectures. No API or Kconfig
change. The condition changes only for a size on a cluster boundary: a size of
0 or a size inside a cluster takes the same path as before. A write that fails
on a full volume no longer destroys the cluster: on master the data is zeroed
before the allocation fails. No documentation describes this behaviour, so
there is no documentation change.
## Testing
A test program on a RAM disk, FAT12 (1024 byte clusters), FAT16 (2048) and
FAT32 (512). It first fills each volume with 0xa5, so a cluster that is not
zeroed does not read as zero by accident. It writes past EOF three ways: on a
new handle, on the handle that wrote the file with the last sector dirty in its
buffer, and with a second handle that has the last sector in its buffer. It
reads the old data, the gap and the new data on that handle, on a new handle
and after a remount, then compares the number of free clusters. The test is not
in nuttx-apps.
qemu-armv8a:nsh (flat) under HVF:
| Old size | Cases | master | this PR |
|---|---|---|---|
| On a cluster boundary (1, 2, 3 clusters; gap of 1 byte, 100 bytes, to the
next boundary, over several clusters; long write; aligned write of 2 clusters)
| 81 | 81 FAIL | 81 pass |
| On a cluster boundary, the write fails on a full volume | 3 | 3 FAIL | 3
pass |
| One byte below a boundary | 27 | 27 pass | 27 pass |
| One byte above a boundary | 27 | 27 pass | 27 pass |
| In the middle of a cluster | 27 | 27 pass | 27 pass |
| Size 0 | 27 | 27 pass | 27 pass |
Each failure reads as `data at 1024 is 0x00, want 0x3b`, also after the
remount.
Other runs of this PR, all pass:
- qemu-armv8a:nsh with `CONFIG_FAT_FORCE_INDIRECT`: 192 of 192.
- qemu-armv8a:knsh (kernel, HVF, `CONFIG_RAM_SIZE` raised to hold the RAM
disks): 192 of 192.
- rv-virt:knsh64 (kernel, TCG) and mps2-an500:knsh (protected, TCG): FAT12
only, 64 of 64 each. These configs have 2 MiB of RAM.
- ostest exits with status 0 on qemu-armv8a:nsh, qemu-armv8a:knsh and
rv-virt:knsh64.
A random test against a model of the file: 3000 operations on each FAT type,
with reads and remounts. A version that changes the size only by recreating the
file fails on all three FAT types on master and passes on this PR (19 writes
past EOF from a boundary). A version with `ftruncate()` stops early on master
and on this PR because of other `ftruncate()` bugs, which #21462 addresses.
Merged with #21462 at 04f0dcd13b8, it passes on all three FAT types (73 writes
past EOF from a boundary). The same merge without this PR fails on all three.
There is no merge conflict with #21462.
Builds with `-Wno-cpp -Werror` and `CONFIG_FS_FAT=y`: qemu-armv8a:nsh,
mps2-an500:nsh (also with `CONFIG_FS_LARGEFILE`), rv-virt:nsh64 (flat);
mps2-an500:knsh, rv-virt:pnsh64, fvp-armv8r:pnsh (protected); qemu-armv8a:knsh,
qemu-armv7a:knsh, rv-virt:knsh64 (kernel).
Not run: real storage, sector sizes other than 512, FAT16 and FAT32 on
RISC-V and arm, and master in the kernel and protected builds.
The runs and builds are on 134ba33ca70. The branch is now rebased on
6b9b216ff30 (no change in fs/fat).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
