Issue about data consistency in fuse-exfat

[Daniel-Xu-1990]

Hi @relan ,

A data consistency problem is happened, when a picture on fuse-exfat is copied to an other path. After copied, the picture lost some data (set as 0) and it looks like a damaged picture.

After code review, I find it does not care the real bytes which are read from device in exfat_generic_pread(). If only part of cluster is read and an IO error occurs, exfat-fuse still reduce the remainder with a cluster size, which means it will tell kernel all data is filled but it did not.

So, I want to discuss whether the real bytes should be checked in exfat_generic_pread() and the other interfaces?

Maybe it should be fixed like this.

@@ -435,11 +436,10 @@ ssize_t exfat_generic_pread(const struct exfat* ef, struct exfat_node* node,
                        return -EIO;
                }
                lsize = MIN(CLUSTER_SIZE(*ef->sb) - loffset, remainder);
-               if (exfat_pread(ef->dev, bufp, lsize,
-                                       exfat_c2o(ef, cluster) + loffset) < 0)
-               {
-                       exfat_error("failed to read cluster %#x", cluster);
-                       return -EIO;
+               ret = exfat_pread(ef->dev, bufp, lsize, exfat_c2o(ef, cluster) + loffset);
+               if (ret != (ssize_t)lsize) {
+                       exfat_error("failed to read cluster %#x, ret %zd remainder %ld", cluster, ret, remainder);
+                       break;
                }
                bufp += lsize;
                loffset = 0;
@@ -448,7 +448,7 @@ ssize_t exfat_generic_pread(const struct exfat* ef, struct exfat_node* node,
        }
        if (!(node->attrib & EXFAT_ATTRIB_DIR) && !ef->ro && !ef->noatime)
                exfat_update_atime(node);
-       return MIN(size, node->size - uoffset) - remainder;
+       return (MIN(size, node->size - uoffset) - remainder > 0) ? MIN(size, node->size - uoffset) - remainder : -EIO;
 }

[relan]

Hi, @Daniel-Xu-1990!

I find it does not care the real bytes which are read from device in exfat_generic_pread(). If only part of cluster is read and an IO error occurs, exfat-fuse still reduce the remainder with a cluster size, which means it will tell kernel all data is filled but it did not.

In the case of an error pread() would return -1 and this condition

           if (exfat_pread(ef->dev, bufp, lsize,
                                   exfat_c2o(ef, cluster) + loffset) < 0)

would fail.

Hypothetically, if the file metadata is corrupt and it's data is beyond the device boundary, pread() can successfully read less bytes than lsize. Can this be your case?

[Daniel-Xu-1990]

Hi, @Daniel-Xu-1990!

I find it does not care the real bytes which are read from device in exfat_generic_pread(). If only part of cluster is read and an IO error occurs, exfat-fuse still reduce the remainder with a cluster size, which means it will tell kernel all data is filled but it did not.

In the case of an error pread() would return -1 and this condition

           if (exfat_pread(ef->dev, bufp, lsize,
                                   exfat_c2o(ef, cluster) + loffset) < 0)

would fail.

Hypothetically, if the file metadata is corrupt and it's data is beyond the device boundary, pread() can successfully read less bytes than lsize. Can this be your case?

In my case, some unexpected BIO errors may occur under low-memory, then only parts of data is read. And based on the POSIX specifications, the result of read() should be checked with 'lsize' rather than zero?