overlayfs v18: readdir.c loses errors and can sync the wrong directory #179

Follow-up from #177. These bugs come with the out-of-tree overlayfs v18 code, so they aren't specific to S3L. #177 deliberately imported the code unchanged rather than fork one copy.

Where

fs/overlayfs/readdir.c has the same blob (4c18abfa) on these kernel branches, plus their topic branches:

• ambarella-s3l
• ingenic-t23
• ingenic-t31
• ti-dm36x
• xiongmai-xm530

The fix should go to all five, so the copies stay identical. Other branches carry a different readdir.c and haven't been compared, so they may or may not have the same bugs.

Line numbers are from ambarella-s3l at 0e5f7083d.

Bugs

1. ovl_dir_read() always returns 0 (readdir.c:188). The error collected in err is lost after fput(). A failed read or allocation leaves a partial list that callers treat as complete. ovl_check_empty_dir could then pass on a non-empty directory, and rmdir would write a whiteout that hides a lower directory persistently.

Likely fix: return err;.

2. ovl_dir_fsync() leaves an ERR_PTR in od->realfile (readdir.c:402). If ovl_path_open() fails, the error pointer stays in the field. The next fsync skips the reopen and uses it, and ovl_dir_release() passes it to fput().

Likely fix: open into a local variable and assign it only on success.

3. ovl_fill_upper() doesn't set rdd->err on allocation failure (readdir.c:166). It returns -ENOMEM directly, while ovl_fill_lower() records the error. With rdd->err still 0 and rdd->count at 1 or more, the do/while in ovl_dir_read() can retry at the same position for as long as the allocation keeps failing. This hasn't been traced through vfs_readdir, so please confirm it before fixing.

4. ovl_dir_fsync() can sync the lower directory (readdir.c:398). The upper directory is only opened when od->realfile is NULL, but that field can still hold the lower directory from ovl_dir_open(). If the directory is copied up while the descriptor stays open, fsync targets the lower file rather than the upper directory receiving the changes.

The fix needs more care than 1–3, because it must not disturb the descriptor's iteration state.

Status

Found by code reading (a review of #177 and Qodo's review on the same PR). None of this has been reproduced on a camera, and no fix has been built. Testing should cover rmdir and readdir on a merged directory, on at least one affected board.

Fixing 1 changes behaviour: readdir and rmdir start returning errors where they used to succeed on a partial list.

Not covered here

• Non-atomic copy-up and removal (copy_up.c:192, dir.c:378). A power cut can leave a partial upper file or bring back a deleted one. That's a v18 design limit, and fixing it means a much larger backport of the later overlayfs design.
• remount,ro on the upper mount. Overlayfs holds mnt_want_write on it (super.c:608), which affects quiesce_overlay in firmware's sysupgrade. It's being tested separately, as asked on ambarella: s3l: add overlayfs (v18), as on the T31 kernel #177.
• Lower-priority findings from Qodo's review on ambarella: s3l: add overlayfs (v18), as on the T31 kernel #177: whiteout failure after a rename, ovl_link ownership and failed-link cleanup, xattr handling in copy-up, and a leak on failed mount-option parsing. These haven't been verified.