[BUG] smb: client: data loss - writeback subrequest is dropped, not retried, when the AEAD allocation fails

Hello,

A large buffered write to an encrypted SMB3 mount can lose data. Every
write() returns success but one or more wsize-sized ranges never reach
the server. ENOMEM is reported once by whichever of fsync() or close()
comes first. On the server a lost range inside the file reads back as
zeroes; a lost range at the end leaves the file short.

We noticed the corruption in June, after upgrading from 6.1 to 6.12.53.
Everything measured below is from a reproducer.


The bug
=======

1. Every encrypted write makes one contiguous allocation for the crypto
   request, in smb2_aead_req_alloc() (fs/smb/client/smb2ops.c):

p = kzalloc(len, GFP_NOFS);
if (!p)
return ERR_PTR(-ENOMEM);

   At wsize=4 MiB with 4 KiB pages num_sgs is 1027. The scatterlist
   array dominates len, so the request is order-4.

2. On failure -ENOMEM reaches smb2_async_writev()
   (fs/smb/client/smb2pdu.c).

3. -ENOMEM is not classified as retryable. On 7.x smb2_async_writev()
   sets NETFS_SREQ_NEED_RETRY only for is_replayable_error()
   (fs/smb/client/cifsglob.h), which is -EAGAIN and -ECONNABORTED. On
   6.12.y netfs makes the same test in
   netfs_write_subrequest_terminated() and retries only -EAGAIN.

4. fs/netfs/write_collect.c marks the subrequest NETFS_SREQ_FAILED,
   records the error on the mapping and does not resend the range.


Affected
========

Reproduced on kernel version:
- v6.12.53
- v7.1.5
- mainline bd5f485f3f02

It needs:
- 4 KiB pages
- an encrypted mount
- buffered I/O (cache=strict or cache=loose)
- and the default wsize=4 MiB.

The classification is old and has never covered -ENOMEM. What changed is
how reachable the allocation failure is:

- before v6.3 kmalloc(len, GFP_ATOMIC) no fallback
- v6.3 onward kvzalloc() vmalloc fallback
- 998a67b95468 kzalloc(len, GFP_NOFS) no fallback

998a67b95468 ("smb: client: fix crypto buffers in non-linear memory")
shipped in 6.6.112+, 6.12.53+, 6.17.3+ and 6.18+. These stable upgrades
took the fallback away:
- 6.6.111  -> 6.6.112
- 6.12.52  -> 6.12.53


Reproducer
==========

1) Samba share on /srv/share, exported with

smb encrypt = required
smb2 max write = 4194304
strict allocate = yes

   mounted on /mnt/cifs with

-o vers=3.1.1,seal,cache=strict,wsize=4194304

2) The writer, cc -O2 -o writer writer.c:

#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

int main(void)
{
static unsigned char buf[1 << 20];
size_t n = 0, total = 192u << 20;
ssize_t rc;
int fd, r;

memset(buf, 0xa5, sizeof buf);
fd = open("/mnt/cifs/data.bin",
 O_WRONLY | O_CREAT | O_TRUNC, 0600);
if (fd < 0) {
perror("open");
return 1;
}
while (n < total) {
rc = write(fd, buf, sizeof buf);
if (rc <= 0) {
printf("write=%zd errno=%d\n", rc, errno);
break;
}
n += (size_t)rc;
}
printf("wrote=%zu\n", n);
r = fsync(fd);
printf("fsync=%d errno=%d\n", r, r ? errno : 0);
r = close(fd);
printf("close=%d errno=%d\n", r, r ? errno : 0);
return 0;
}

3) Inject during the write:

F=/sys/kernel/debug/fail_page_alloc
echo 4  > $F/min-order
echo N  > $F/ignore-gfp-wait
echo -1 > $F/times
trap 'echo 0 > $F/probability' EXIT INT TERM
echo 50 > $F/probability
./writer
echo 0  > $F/probability

4) Check the file on the server, not through the mount, which still
   serves the original pages from cache. The file is 0xa5, so every
   zero byte is lost data:

cd /srv/share
ls -l data.bin # short if the tail was dropped
tr -dc '\0' <data.bin | wc -c # bytes lost as zeroes
od -A d -t x1 data.bin | grep -B1 '00 00 00 00'


Result
======

Three runs on mainline bd5f485f3f02. Every write() returned success
and fsync() returned -1 ENOMEM:

run 1 full length 80 MiB lost 20 chunks
run 2 8 MiB short 92 MiB lost 23 chunks
run 3 full length 96 MiB lost 24 chunks

Every zero run starts and ends on a wsize boundary and the chunk count
matches the injected failures in crypt_message exactly. Raising min-order
to 5 misses this allocation entirely: 206 injections land in
netfs_alloc_folioq_buffer instead and nothing is lost.

Dropping the fsync() from the writer moves the ENOMEM to close(). v7.1.5
behaved the same, losing 104, 88 and 92 MiB.

The error is reported but the data is already gone. With the stream
failed netfs_collect_write_results() advances collected_to over the
untransferred range and ends the folios without redirtying them;
fs/netfs/write_collect.c has no redirty call at all. An application that
checks every return code still cannot rewrite the lost range. On the
issue side the same -ENOMEM is treated as transient and redirtied
(fs/netfs/write_issue.c).

I would expect a transient -ENOMEM to be retried rather than treated as a
permanent failure of the range, or failing that for folios not to be
cleaned when nothing was transferred.

Happy to test patches or add trace detail.

Thanks,
Mihail Leoca