[PATCH 0/3] btrfs-progs: receive: fix a silent data loss bug with encoded writes

[PATCH 0/3] btrfs-progs: receive: fix a silent data loss bug with encoded writes

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When using a v2 stream, with encoded writes, if the receiver fails to
perform an encoded write, it fallbacks to decompressing the data and then
write it using regular buffered IO. However that logic is currenty buggy,
and it can result in writing less data than expected or no data at all.
This results in a silent data loss.

Patch 3/3 fixes that bug and has all the details about how/why it happens,
while previous patches just added debug messages to the callbacks for
encoded writes and fallocate, which are currently missing and are very
useful for debugging.

Filipe Manana (3):
  btrfs-progs: receive: add debug messages when processing encoded writes
  btrfs-progs: receive: add debug messages when processing fallocate
  btrfs-progs: receive: fix silent data loss after fall back from encoded write

 cmds/receive.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

-- 
2.35.1

[PATCH 1/3] btrfs-progs: receive: add debug messages when processing encoded writes

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Unlike for commands from the v1 stream, we have no debug messages logged
when processing encoded write commands, which makes it harder to debug
issues.

So add log messages, when the log verbosity level is >= 3, for encoded
write commands, mentioning the value of all fields and also log when we
fallback from an encoded write to the decompress and write approach.

The log messages look like this:

  encoded_write f3 - offset=33792, len=4096, unencoded_offset=33792, unencoded_file_len=31744, unencoded_len=65536, compression=1, encryption=0
  encoded_write f3 - falling back to decompress and write due to errno 22 ("Invalid argument")

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 cmds/receive.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/cmds/receive.c b/cmds/receive.c
index 6f475544..b75a5634 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -1246,6 +1246,13 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,
 		.encryption = encryption,
 	};
 
+	if (bconf.verbose >= 3)
+		fprintf(stderr,
+"encoded_write %s - offset=%llu, len=%llu, unencoded_offset=%llu, unencoded_file_len=%llu, unencoded_len=%llu, compression=%u, encryption=%u\n",
+			path, offset, len, unencoded_offset, unencoded_file_len,
+			unencoded_len, compression, encryption);
+
+
 	if (encryption) {
 		error("encoded_write: encryption not supported");
 		return -EOPNOTSUPP;
@@ -1271,6 +1278,10 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,
 			error("encoded_write: writing to %s failed: %m", path);
 			return ret;
 		}
+		if (bconf.verbose >= 3)
+			fprintf(stderr,
+"encoded_write %s - falling back to decompress and write due to errno %d (\"%m\")\n",
+				path, errno);
 	}
 
 	return decompress_and_write(rctx, data, offset, len, unencoded_file_len,
-- 
2.35.1

Re: [PATCH 1/3] btrfs-progs: receive: add debug messages when processing encoded writes

[Boris Burkov]

On Tue, Nov 15, 2022 at 04:25:24PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Unlike for commands from the v1 stream, we have no debug messages logged
> when processing encoded write commands, which makes it harder to debug
> issues.
> 
> So add log messages, when the log verbosity level is >= 3, for encoded
> write commands, mentioning the value of all fields and also log when we
> fallback from an encoded write to the decompress and write approach.
> 
> The log messages look like this:
> 
>   encoded_write f3 - offset=33792, len=4096, unencoded_offset=33792, unencoded_file_len=31744, unencoded_len=65536, compression=1, encryption=0
>   encoded_write f3 - falling back to decompress and write due to errno 22 ("Invalid argument")
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Boris Burkov <boris@bur.io>
> ---
>  cmds/receive.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/cmds/receive.c b/cmds/receive.c
> index 6f475544..b75a5634 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -1246,6 +1246,13 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,
>  		.encryption = encryption,
>  	};
>  
> +	if (bconf.verbose >= 3)
> +		fprintf(stderr,
> +"encoded_write %s - offset=%llu, len=%llu, unencoded_offset=%llu, unencoded_file_len=%llu, unencoded_len=%llu, compression=%u, encryption=%u\n",
> +			path, offset, len, unencoded_offset, unencoded_file_len,
> +			unencoded_len, compression, encryption);
> +
> +
>  	if (encryption) {
>  		error("encoded_write: encryption not supported");
>  		return -EOPNOTSUPP;
> @@ -1271,6 +1278,10 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,
>  			error("encoded_write: writing to %s failed: %m", path);
>  			return ret;
>  		}
> +		if (bconf.verbose >= 3)
> +			fprintf(stderr,
> +"encoded_write %s - falling back to decompress and write due to errno %d (\"%m\")\n",
> +				path, errno);
>  	}
>  
>  	return decompress_and_write(rctx, data, offset, len, unencoded_file_len,
> -- 
> 2.35.1
> 

[PATCH 2/3] btrfs-progs: receive: add debug messages when processing fallocate

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Unlike for commands from the v1 stream, we have no debug messages logged
when processing fallocate commands, which makes it harder to debug issues.

So add log messages, when the log verbosity level is >= 3, for fallocate
commands, mentioning the value of all fields.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 cmds/receive.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/cmds/receive.c b/cmds/receive.c
index b75a5634..e6f1aeab 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -1296,6 +1296,11 @@ static int process_fallocate(const char *path, int mode, u64 offset, u64 len,
 	struct btrfs_receive *rctx = user;
 	char full_path[PATH_MAX];
 
+	if (bconf.verbose >= 3)
+		fprintf(stderr,
+			"fallocate %s - offset=%llu, len=%llu, mode=%d\n",
+			path, offset, len, mode);
+
 	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
 	if (ret < 0) {
 		error("fallocate: path invalid: %s", path);
-- 
2.35.1

Re: [PATCH 2/3] btrfs-progs: receive: add debug messages when processing fallocate

[Boris Burkov]

On Tue, Nov 15, 2022 at 04:25:25PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Unlike for commands from the v1 stream, we have no debug messages logged
> when processing fallocate commands, which makes it harder to debug issues.
> 
> So add log messages, when the log verbosity level is >= 3, for fallocate
> commands, mentioning the value of all fields.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Boris Burkov <boris@bur.io>
> ---
>  cmds/receive.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/cmds/receive.c b/cmds/receive.c
> index b75a5634..e6f1aeab 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -1296,6 +1296,11 @@ static int process_fallocate(const char *path, int mode, u64 offset, u64 len,
>  	struct btrfs_receive *rctx = user;
>  	char full_path[PATH_MAX];
>  
> +	if (bconf.verbose >= 3)
> +		fprintf(stderr,
> +			"fallocate %s - offset=%llu, len=%llu, mode=%d\n",
> +			path, offset, len, mode);
> +
>  	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
>  	if (ret < 0) {
>  		error("fallocate: path invalid: %s", path);
> -- 
> 2.35.1
> 

[PATCH 3/3] btrfs-progs: receive: fix silent data loss after fall back from encoded write

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When attempting an encoded write, if it fails for some specific reason
like -EINVAL (when an offset is not sector size aligned) or -ENOSPC, we
then fallback into decompressing the data and writing it using regular
buffered IO. This logic however is not correct, one of the reasons is
that it assumes the encoded offset is smaller than the unencoded file
length and that they can be compared, but one is an offset and the other
is a length, not an end offset, so they can't be compared to get correct
results. This bad logic will often result in not copying all data, or even
no data at all, resulting in a silent data loss. This is easily seen in
with the following reproducer:

   $ cat test.sh
   #!/bin/bash

   DEV=/dev/sdj
   MNT=/mnt/sdj

   umount $DEV &> /dev/null
   mkfs.btrfs -f $DEV > /dev/null
   mount -o compress $DEV $MNT

   # File foo has a size of 33K, not aligned to the sector size.
   xfs_io -f -c "pwrite -S 0xab 0 33K" $MNT/foo

   xfs_io -f -c "pwrite -S 0xcd 0 64K" $MNT/bar

   # Now clone the first 32K of file bar into foo at offset 0.
   xfs_io -c "reflink $MNT/bar 0 0 32K" $MNT/foo

   # Snapshot the default subvolume and create a full send stream (v2).
   btrfs subvolume snapshot -r $MNT $MNT/snap

   btrfs send --compressed-data -f /tmp/test.send $MNT/snap

   echo -e "\nFile bar in the original filesystem:"
   od -A d -t x1 $MNT/snap/bar

   umount $MNT
   mkfs.btrfs -f $DEV > /dev/null
   mount $DEV $MNT

   echo -e "\nReceiving stream in a new filesystem..."
   btrfs receive -f /tmp/test.send $MNT

   echo -e "\nFile bar in the new filesystem:"
   od -A d -t x1 $MNT/snap/bar

   umount $MNT

Running the test without this patch:

   $ ./test.sh
   (...)
   File bar in the original filesystem:
   0000000 cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd
   *
   0065536

   Receiving stream in a new filesystem...
   At subvol snap

   File bar in the new filesystem:
   0000000 cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd
   *
   0033792

We end up with file bar having less data, and a smaller size, than in the
original filesystem.

This happens because when processing file bar, send issues the following
commands:

   clone bar - source=foo source offset=0 offset=0 length=32768
   write bar - offset=32768 length=1024
   encoded_write bar - offset=33792, len=4096, unencoded_offset=33792, unencoded_file_len=31744, unencoded_len=65536, compression=1, encryption=0

The first 32K are cloned from file foo, as that file ranged is shared
between the files.

Then there's a regular write operation for the file range [32K, 33K[,
since file foo has different data from bar for that file range.

Finally for the remainder of file bar, the send side issues an encoded
write since the extent is compressed in the source filesystem, for the
file offset 33792 (33K), remaining 31K of data. The receiver will try the
encoded write, but that fails with -EINVAL since the offset 33K is not
sector size aligned, so it will fallback to decompressing the data and
writing it using regular buffered writes. However that results in doing
no writes at decompress_and_write() because 'pos' is initialized to the
value of 33K (unencoded_offset) and unencoded_file_len is 31K, so the
while loop has no iterations.

Another case where we can fallback to decompression plus regular buffered
writes is when the destination filesystem has a sector size larger then
the sector size of the source filesystem (for example when the source
filesystem is on x86_64 with a 4K sector size and the destination
filesystem is PowerPC with a 64K sector size). In that scenario encoded
write attempts wil fail with -EINVAL due to offsets not being aligned with
the sector size of the destination filesystem, and the receive will
attempt the fallback of decompressing the buffer and writing the
decompressed using regular buffered IO.

Fix this by tracking the number of written bytes instead, and increment
it, and the unencoded offset, after each write.

Fixes: d20e759fc917 ("btrfs-progs: receive: encoded_write fallback to explicit decode and write")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 cmds/receive.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/cmds/receive.c b/cmds/receive.c
index e6f1aeab..0db66ee5 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -1155,10 +1155,9 @@ static int decompress_and_write(struct btrfs_receive *rctx,
 				u32 compression)
 {
 	int ret = 0;
-	size_t pos;
-	ssize_t w;
 	char *unencoded_data;
 	int sector_shift = 0;
+	u64 written = 0;
 
 	unencoded_data = calloc(unencoded_len, 1);
 	if (!unencoded_data) {
@@ -1209,17 +1208,19 @@ static int decompress_and_write(struct btrfs_receive *rctx,
 		goto out;
 	}
 
-	pos = unencoded_offset;
-	while (pos < unencoded_file_len) {
-		w = pwrite(rctx->write_fd, unencoded_data + pos,
-			   unencoded_file_len - pos, offset);
+	while (written < unencoded_file_len) {
+		ssize_t w;
+
+		w = pwrite(rctx->write_fd, unencoded_data + unencoded_offset,
+			   unencoded_file_len - written, offset);
 		if (w < 0) {
 			ret = -errno;
 			error("writing unencoded data failed: %m");
 			goto out;
 		}
-		pos += w;
+		written += w;
 		offset += w;
+		unencoded_offset += w;
 	}
 out:
 	free(unencoded_data);
-- 
2.35.1

Re: [PATCH 3/3] btrfs-progs: receive: fix silent data loss after fall back from encoded write

[Boris Burkov]

On Tue, Nov 15, 2022 at 04:25:26PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When attempting an encoded write, if it fails for some specific reason
> like -EINVAL (when an offset is not sector size aligned) or -ENOSPC, we
> then fallback into decompressing the data and writing it using regular
> buffered IO. This logic however is not correct, one of the reasons is
> that it assumes the encoded offset is smaller than the unencoded file
> length and that they can be compared, but one is an offset and the other
> is a length, not an end offset, so they can't be compared to get correct
> results. This bad logic will often result in not copying all data, or even
> no data at all, resulting in a silent data loss. This is easily seen in
> with the following reproducer:
> 
>    $ cat test.sh
>    #!/bin/bash
> 
>    DEV=/dev/sdj
>    MNT=/mnt/sdj
> 
>    umount $DEV &> /dev/null
>    mkfs.btrfs -f $DEV > /dev/null
>    mount -o compress $DEV $MNT
> 
>    # File foo has a size of 33K, not aligned to the sector size.
>    xfs_io -f -c "pwrite -S 0xab 0 33K" $MNT/foo
> 
>    xfs_io -f -c "pwrite -S 0xcd 0 64K" $MNT/bar
> 
>    # Now clone the first 32K of file bar into foo at offset 0.
>    xfs_io -c "reflink $MNT/bar 0 0 32K" $MNT/foo
> 
>    # Snapshot the default subvolume and create a full send stream (v2).
>    btrfs subvolume snapshot -r $MNT $MNT/snap
> 
>    btrfs send --compressed-data -f /tmp/test.send $MNT/snap
> 
>    echo -e "\nFile bar in the original filesystem:"
>    od -A d -t x1 $MNT/snap/bar
> 
>    umount $MNT
>    mkfs.btrfs -f $DEV > /dev/null
>    mount $DEV $MNT
> 
>    echo -e "\nReceiving stream in a new filesystem..."
>    btrfs receive -f /tmp/test.send $MNT
> 
>    echo -e "\nFile bar in the new filesystem:"
>    od -A d -t x1 $MNT/snap/bar
> 
>    umount $MNT
> 
> Running the test without this patch:
> 
>    $ ./test.sh
>    (...)
>    File bar in the original filesystem:
>    0000000 cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd
>    *
>    0065536
> 
>    Receiving stream in a new filesystem...
>    At subvol snap
> 
>    File bar in the new filesystem:
>    0000000 cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd
>    *
>    0033792
> 
> We end up with file bar having less data, and a smaller size, than in the
> original filesystem.
> 
> This happens because when processing file bar, send issues the following
> commands:
> 
>    clone bar - source=foo source offset=0 offset=0 length=32768
>    write bar - offset=32768 length=1024
>    encoded_write bar - offset=33792, len=4096, unencoded_offset=33792, unencoded_file_len=31744, unencoded_len=65536, compression=1, encryption=0
> 
> The first 32K are cloned from file foo, as that file ranged is shared
> between the files.
> 
> Then there's a regular write operation for the file range [32K, 33K[,
> since file foo has different data from bar for that file range.
> 
> Finally for the remainder of file bar, the send side issues an encoded
> write since the extent is compressed in the source filesystem, for the
> file offset 33792 (33K), remaining 31K of data. The receiver will try the
> encoded write, but that fails with -EINVAL since the offset 33K is not
> sector size aligned, so it will fallback to decompressing the data and
> writing it using regular buffered writes. However that results in doing
> no writes at decompress_and_write() because 'pos' is initialized to the
> value of 33K (unencoded_offset) and unencoded_file_len is 31K, so the
> while loop has no iterations.
> 
> Another case where we can fallback to decompression plus regular buffered
> writes is when the destination filesystem has a sector size larger then
> the sector size of the source filesystem (for example when the source
> filesystem is on x86_64 with a 4K sector size and the destination
> filesystem is PowerPC with a 64K sector size). In that scenario encoded
> write attempts wil fail with -EINVAL due to offsets not being aligned with
> the sector size of the destination filesystem, and the receive will
> attempt the fallback of decompressing the buffer and writing the
> decompressed using regular buffered IO.
> 
> Fix this by tracking the number of written bytes instead, and increment
> it, and the unencoded offset, after each write.

Thank you for catching and fixing this.

> 
> Fixes: d20e759fc917 ("btrfs-progs: receive: encoded_write fallback to explicit decode and write")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Boris Burkov <boris@bur.io>
> ---
>  cmds/receive.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/cmds/receive.c b/cmds/receive.c
> index e6f1aeab..0db66ee5 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -1155,10 +1155,9 @@ static int decompress_and_write(struct btrfs_receive *rctx,
>  				u32 compression)
>  {
>  	int ret = 0;
> -	size_t pos;
> -	ssize_t w;
>  	char *unencoded_data;
>  	int sector_shift = 0;
> +	u64 written = 0;
>  
>  	unencoded_data = calloc(unencoded_len, 1);
>  	if (!unencoded_data) {
> @@ -1209,17 +1208,19 @@ static int decompress_and_write(struct btrfs_receive *rctx,
>  		goto out;
>  	}
>  
> -	pos = unencoded_offset;
> -	while (pos < unencoded_file_len) {
> -		w = pwrite(rctx->write_fd, unencoded_data + pos,
> -			   unencoded_file_len - pos, offset);
> +	while (written < unencoded_file_len) {
> +		ssize_t w;
> +
> +		w = pwrite(rctx->write_fd, unencoded_data + unencoded_offset,
> +			   unencoded_file_len - written, offset);
>  		if (w < 0) {
>  			ret = -errno;
>  			error("writing unencoded data failed: %m");
>  			goto out;
>  		}
> -		pos += w;
> +		written += w;
>  		offset += w;
> +		unencoded_offset += w;
>  	}
>  out:
>  	free(unencoded_data);
> -- 
> 2.35.1
> 

Re: [PATCH 0/3] btrfs-progs: receive: fix a silent data loss bug with encoded writes

[Christoph Anton Mitterer]

Hey.

I recently sent | received a lot of previous data, thus asking:

What exactly does encoded write mean? Is one *only* affected when ones
used compression - respectively if one DID NOT do any filesystem
compression (i.e. compress mount option)... can one be sure to be safe?

Thanks,
Chris.


[0] https://lore.kernel.org/linux-btrfs/cover.1660690698.git.osandov@fb.com/

Re: [PATCH 0/3] btrfs-progs: receive: fix a silent data loss bug with encoded writes

[Filipe Manana]

On Tue, Nov 15, 2022 at 4:37 PM Christoph Anton Mitterer
<calestyo@scientia.org> wrote:
>
> Hey.
>
> I recently sent | received a lot of previous data, thus asking:
>
> What exactly does encoded write mean?

Right now, it means a write operation where the data is compressed.
I.e. there was no decompression at the sender side.

> Is one *only* affected when ones
> used compression - respectively if one DID NOT do any filesystem
> compression (i.e. compress mount option)... can one be sure to be safe?

If you haven't used 'btrfs send' with the --compressed-data option or
you are sure you don't have any compressed files, then you're fine.
Otherwise check your files by comparing them between the sending side
and the receiving side.

>
> Thanks,
> Chris.
>
>
> [0] https://lore.kernel.org/linux-btrfs/cover.1660690698.git.osandov@fb.com/

Re: [PATCH 0/3] btrfs-progs: receive: fix a silent data loss bug with encoded writes

[Christoph Anton Mitterer]

On Tue, 2022-11-15 at 16:47 +0000, Filipe Manana wrote:
> > Is one *only* affected when ones
> > used compression - respectively if one DID NOT do any filesystem
> > compression (i.e. compress mount option)... can one be sure to be
> > safe?
> 
> If you haven't used 'btrfs send' with the --compressed-data option or
> you are sure you don't have any compressed files, then you're fine.

Thanks a lot... so all good for me. :-)

But still, as I wrote in the other mail,... other people might be
affected... and it would be reeeeally nice if there was some good way
for them to get alerted about such cases.


Thanks,
Chris.

Re: [PATCH 0/3] btrfs-progs: receive: fix a silent data loss bug with encoded writes

[Filipe Manana]

On Tue, Nov 15, 2022 at 4:53 PM Christoph Anton Mitterer
<calestyo@scientia.org> wrote:
>
> On Tue, 2022-11-15 at 16:47 +0000, Filipe Manana wrote:
> > > Is one *only* affected when ones
> > > used compression - respectively if one DID NOT do any filesystem
> > > compression (i.e. compress mount option)... can one be sure to be
> > > safe?
> >
> > If you haven't used 'btrfs send' with the --compressed-data option or
> > you are sure you don't have any compressed files, then you're fine.
>
> Thanks a lot... so all good for me. :-)
>
> But still, as I wrote in the other mail,... other people might be
> affected... and it would be reeeeally nice if there was some good way
> for them to get alerted about such cases.

There will always be users who'll miss such alerts, many don't read
all the emails in this list, many are not even subscribed to the
mailing list, etc.
Do you have examples of other projects that have an effective alert system?

>
>
> Thanks,
> Chris.

Re: [PATCH 0/3] btrfs-progs: receive: fix a silent data loss bug with encoded writes

[Christoph Anton Mitterer]

On Wed, 2022-11-16 at 10:50 +0000, Filipe Manana wrote:
> There will always be users who'll miss such alerts, many don't read
> all the emails in this list, many are not even subscribed to the
> mailing list, etc.

Sure... and it will never be possible to have a system which really
reaches 100% of the desired users.

But this isn't only the case for a something that notices about data
corruption issues, it's also the case for any other warning system:
- civil defense via sirens (people may life to far away, wear
  headphones with loud music, be deaf, sleep with earmuffs) via apps
  (they often simply fail or are overloaded, people may not have a
  smartphone at all, it maybe powered off)
- software security advisories (again, people may not be subscribed to
  such mailing lists, may not run upgrades regularly respectively check
  for security updates in some automated fashion, or attackers may try
  to specifically block such information from certain people)
- etc. pp.
- and even *if* the information reaches someone, it's still not 
  guaranteed that he cares about or understands it well enough


I don't think the goal is ever about reaching everyone - the goal is
having a simple way of reaching those who care.

Because whether it's storage farm admins who keep important scientific
data on btrfs or some end user who values his precious collection of
pictures, etc. ... it would be quite good for them to be able to react
in cases of (especially silent) data corruption.

And I should emphasis, that this is in no way targeted for lazy people
who don't make backups.
I for example do have some rather sophisticated backup strategy, but if
there's silent data corruption it's often quite hard to tell whether
the data is still valid, even if one does things like storing hash sums
of them (and verifying these of some 10TB of data already takes several
days).


> Do you have examples of other projects that have an effective alert
> system?

Well, as I've said, not at the filesystem level. But it's e.g. common
practise for security incidents.


Upstream developers put quite some effort into finding and fixing such
bugs, which is appreciated, but IMO that is a bit ridiculed by the fact
that people then may still loose their data, just because they never
take notice about such issues, when they'd still have valid backups.


Cheers,
Chris.

there should be some better way to inform interested people about data corruption issues

[Christoph Anton Mitterer]

Hey again.

Something more general:


Just as with the most recent (at least as far as I've noticed) possible
silent data corruption[0], it's IMO really most unfortunate and a
structural problem, that there is no useful way for (interested) end
users to take notice them.


If I weren't following the list a bit - and even there only by chance
and depending on an alerting commit summary - I'd probably never ever
hear about it... yet I might still suffer from it without realising
immediately.

Right now people may still have backups respectively the sources they
btrfs-sent from... but perhaps not so in a year when they possibly
notice the corruption (if at all).


This is really no criticism that such bugs do happen - btrfs is quite
actively developed, so I fully understand that such bugs show up.

But for end users its really awful, if especially for silent corruption
issues there is no alerting mechanism.

And yes I know, other filesystems don't have one either - doesn't make
it better though.



I'd say a simple corruptions announcement mailing list would do:

No other announcements like "new btrfs progs version" (unless that
would fix a data corruption issue). Also usually no announcement for
issues that were 100% user-visible (like general crash when btrfs tries
to mount a fs) OR which have no impact on data consistency (e.g. if a
bug would prevent the "compress" mount option to be considered at all).

Only really serious things that could cause data loss (perhaps
including confidentiality issues, like with fscrypt).

Per issue, one announcement mail with description on what it is, how
likely it happened, if/how one can find out whether one was affected,
how one can fix (or if checks against / recovery from backups are
needed).


Perhaps other filesystems would even want to join in, then mails should
be prefixed with "btrfs: ", "xfs:", "ext4", etc.:



Regards,
Chris.

Re: [PATCH 0/3] btrfs-progs: receive: fix a silent data loss bug with encoded writes

[David Sterba]

On Tue, Nov 15, 2022 at 04:25:23PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When using a v2 stream, with encoded writes, if the receiver fails to
> perform an encoded write, it fallbacks to decompressing the data and then
> write it using regular buffered IO. However that logic is currenty buggy,
> and it can result in writing less data than expected or no data at all.
> This results in a silent data loss.
> 
> Patch 3/3 fixes that bug and has all the details about how/why it happens,
> while previous patches just added debug messages to the callbacks for
> encoded writes and fallocate, which are currently missing and are very
> useful for debugging.
> 
> Filipe Manana (3):
>   btrfs-progs: receive: add debug messages when processing encoded writes
>   btrfs-progs: receive: add debug messages when processing fallocate
>   btrfs-progs: receive: fix silent data loss after fall back from encoded write

Added to devel, thanks.