mirror of
https://github.com/torvalds/linux.git
synced 2026-11-10 17:11:02 +01:00
ntfs: fix volume flag update races
ntfs_set_volume_flags() and ntfs_clear_volume_flags() both read vol->vol_flags outside any lock to compute the new value before handing it to ntfs_write_volume_flags(), which only takes ni->mrec_lock around the actual write. The read-modify-write is therefore not atomic, and two concurrent callers can lose an update: ntfs_sync_fs() may derive a "clean" value from vol->vol_flags while a writer concurrently records an error and sets VOLUME_IS_DIRTY; the locked write then silently overwrites the freshly-set dirty bit. The on-disk volume looks clean despite the recorded errors, so chkdsk will not run on the next mount and corrupted metadata can persist. Fix by moving the read-modify-write inside the mrec_lock: pass the bits to set and to clear separately, and combine them with the current flag state under the lock inside ntfs_write_volume_flags(). The set/clear helpers pass only the bits to modify, not the complete flag state. The bit manipulation is done on CPU-endian values, and the result is converted back to little-endian before storing it. The wrappers keep their signatures so callers are unchanged. Cc: stable@vger.kernel.org Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn> Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com> Reviewed-by: Baolin Liu <liubaolin@kylinos.cn> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
This commit is contained in:
parent
fca746064d
commit
56a64ee0f9
|
|
@ -360,31 +360,45 @@ void ntfs_handle_error(struct super_block *sb)
|
|||
}
|
||||
|
||||
/*
|
||||
* ntfs_write_volume_flags - write new flags to the volume information flags
|
||||
* ntfs_write_volume_flags - apply flag changes to the volume information flags
|
||||
* @vol: ntfs volume on which to modify the flags
|
||||
* @flags: new flags value for the volume information flags
|
||||
* @set_bits: bits to set in the volume information flags
|
||||
* @clear_bits: bits to clear in the volume information flags
|
||||
*
|
||||
* Internal function. You probably want to use ntfs_{set,clear}_volume_flags()
|
||||
* instead (see below).
|
||||
*
|
||||
* Replace the volume information flags on the volume @vol with the value
|
||||
* supplied in @flags. Note, this overwrites the volume information flags, so
|
||||
* make sure to combine the flags you want to modify with the old flags and use
|
||||
* the result when calling ntfs_write_volume_flags().
|
||||
* Combine @set_bits and @clear_bits with the current in-memory flag state and
|
||||
* write the result back. The set/clear helpers pass only the bits to modify,
|
||||
* not the complete flag state. The read-modify-write happens under
|
||||
* ni->mrec_lock so that concurrent set/clear operations cannot lose updates.
|
||||
* All bit manipulation is done on CPU-endian values, and the result is
|
||||
* converted back to little-endian before storing it.
|
||||
*
|
||||
* Return 0 on success and -errno on error.
|
||||
*/
|
||||
static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
|
||||
static int ntfs_write_volume_flags(struct ntfs_volume *vol,
|
||||
const __le16 set_bits, const __le16 clear_bits,
|
||||
const bool skip_if_errors)
|
||||
{
|
||||
struct ntfs_inode *ni = NTFS_I(vol->vol_ino);
|
||||
struct volume_information *vi;
|
||||
struct ntfs_attr_search_ctx *ctx;
|
||||
u16 flags;
|
||||
int err;
|
||||
|
||||
ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
|
||||
le16_to_cpu(vol->vol_flags), le16_to_cpu(flags));
|
||||
mutex_lock(&ni->mrec_lock);
|
||||
if (vol->vol_flags == flags)
|
||||
|
||||
if (skip_if_errors && NVolErrors(vol))
|
||||
goto done;
|
||||
|
||||
flags = le16_to_cpu(vol->vol_flags);
|
||||
flags |= le16_to_cpu(set_bits) & le16_to_cpu(VOLUME_FLAGS_MASK);
|
||||
flags &= ~(le16_to_cpu(clear_bits) & le16_to_cpu(VOLUME_FLAGS_MASK));
|
||||
ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
|
||||
le16_to_cpu(vol->vol_flags), flags);
|
||||
|
||||
if (le16_to_cpu(vol->vol_flags) == flags)
|
||||
goto done;
|
||||
|
||||
ctx = ntfs_attr_get_search_ctx(ni, NULL);
|
||||
|
|
@ -400,7 +414,7 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
|
|||
|
||||
vi = (struct volume_information *)((u8 *)ctx->attr +
|
||||
le16_to_cpu(ctx->attr->data.resident.value_offset));
|
||||
vol->vol_flags = vi->flags = flags;
|
||||
vol->vol_flags = vi->flags = cpu_to_le16(flags);
|
||||
mark_mft_record_dirty(ctx->ntfs_ino);
|
||||
ntfs_attr_put_search_ctx(ctx);
|
||||
done:
|
||||
|
|
@ -421,13 +435,14 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
|
|||
* @flags: flags to set on the volume
|
||||
*
|
||||
* Set the bits in @flags in the volume information flags on the volume @vol.
|
||||
* The bits are combined with the current flag state under the lock in
|
||||
* ntfs_write_volume_flags(), so concurrent updates are not lost.
|
||||
*
|
||||
* Return 0 on success and -errno on error.
|
||||
*/
|
||||
int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
|
||||
{
|
||||
flags &= VOLUME_FLAGS_MASK;
|
||||
return ntfs_write_volume_flags(vol, vol->vol_flags | flags);
|
||||
return ntfs_write_volume_flags(vol, flags, 0, false);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -436,14 +451,27 @@ int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
|
|||
* @flags: flags to clear on the volume
|
||||
*
|
||||
* Clear the bits in @flags in the volume information flags on the volume @vol.
|
||||
* The bits are combined with the current flag state under the lock in
|
||||
* ntfs_write_volume_flags(), so concurrent updates are not lost.
|
||||
*
|
||||
* Return 0 on success and -errno on error.
|
||||
*/
|
||||
int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags)
|
||||
{
|
||||
flags &= VOLUME_FLAGS_MASK;
|
||||
flags = vol->vol_flags & cpu_to_le16(~le16_to_cpu(flags));
|
||||
return ntfs_write_volume_flags(vol, flags);
|
||||
return ntfs_write_volume_flags(vol, 0, flags, false);
|
||||
}
|
||||
|
||||
/*
|
||||
* ntfs_clear_volume_dirty_if_no_errors - clear dirty bit if no errors exist
|
||||
* @vol: ntfs volume whose dirty bit should be cleared
|
||||
*
|
||||
* Check NVolErrors() and clear VOLUME_IS_DIRTY under the same mrec_lock so
|
||||
* ntfs_sync_fs() cannot clear the dirty bit after a concurrent error has been
|
||||
* recorded.
|
||||
*/
|
||||
static int ntfs_clear_volume_dirty_if_no_errors(struct ntfs_volume *vol)
|
||||
{
|
||||
return ntfs_write_volume_flags(vol, 0, VOLUME_IS_DIRTY, true);
|
||||
}
|
||||
|
||||
int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
|
||||
|
|
@ -1926,8 +1954,7 @@ static int ntfs_sync_fs(struct super_block *sb, int wait)
|
|||
return 0;
|
||||
|
||||
/* If there are some dirty buffers in the bdev inode */
|
||||
if (!NVolErrors(vol) &&
|
||||
ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY)) {
|
||||
if (ntfs_clear_volume_dirty_if_no_errors(vol)) {
|
||||
ntfs_warning(sb, "Failed to clear dirty bit in volume information flags. Run chkdsk.");
|
||||
err = -EIO;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user