Re: [Linux-NTFS-Dev] fs/ntfs/mft.c IS_ERR problem
Development moved to https://sourceforge.net/projects/ntfs-3g/
Brought to you by:
antona,
cha0smaster
From: Anton A. <ai...@ca...> - 2008-05-12 07:20:20
|
Hi Julia, On 11 May 2008, at 21:19, Julia Lawall wrote: > The following code, starting at line 2591 in the file fs/ntfs/mft.c > does > not look right: > > m = map_extent_mft_record(base_ni, bit, &ni); > if (IS_ERR(m)) { > ntfs_error(vol->sb, "Failed to map allocated extent " > "mft record 0x%llx.", (long long)bit); > err = PTR_ERR(m); > /* Set the mft record itself not in use. */ > m->flags &= cpu_to_le16( > ~le16_to_cpu(MFT_RECORD_IN_USE)); > > If m satisfies IS_ERR(m) it does not seem correct to dereference it. > Other error handling code in the same function also sets m->flags, > but not > for the same value of m. In this case m has been redefined. > Perhaps the > solution is to save the previous value of m in a temporary variable so > that the flags field can be updated here. Thanks for that! Nice catch. In practice map_extent_mft_record() will only fail if there is not enough memory so it will almost never trigger but nonetheless it needs to be fixed. Below is a quick fix for this. Best regards, Anton -- Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @) Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK Linux NTFS maintainer, http://www.linux-ntfs.org/ Fix dereference of incorrect variable. Thanks to Julia Lawall for catching this bug. Signed-off-by: Anton Altaparmakov <ai...@ca...> diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c index 2ad5c8b..86e5002 100644 --- a/fs/ntfs/mft.c +++ b/fs/ntfs/mft.c @@ -2576,6 +2576,8 @@ mft_rec_already_initialized: flush_dcache_page(page); SetPageUptodate(page); if (base_ni) { + MFT_RECORD *m_tmp; + /* * Setup the base mft record in the extent mft record. This * completes initialization of the allocated extent mft record @@ -2588,11 +2590,11 @@ mft_rec_already_initialized: * attach it to the base inode @base_ni and map, pin, and lock * its, i.e. the allocated, mft record. */ - m = map_extent_mft_record(base_ni, bit, &ni); - if (IS_ERR(m)) { + m_tmp = map_extent_mft_record(base_ni, bit, &ni); + if (IS_ERR(m_tmp)) { ntfs_error(vol->sb, "Failed to map allocated extent " "mft record 0x%llx.", (long long)bit); - err = PTR_ERR(m); + err = PTR_ERR(m_tmp); /* Set the mft record itself not in use. */ m->flags &= cpu_to_le16( ~le16_to_cpu(MFT_RECORD_IN_USE)); @@ -2603,6 +2605,7 @@ mft_rec_already_initialized: ntfs_unmap_page(page); goto undo_mftbmp_alloc; } + BUG_ON(m != m_tmp); /* * Make sure the allocated mft record is written out to disk. * No need to set the inode dirty because the caller is going |