Hi Sergei,
On 15 Oct 2014, at 20:19, Sergei Antonov <saproj@xxxxxxxxx> wrote:
> Implement ntfs_bmap() function for the "bmap" address space operation.
> Now user space programs can send FIBMAP ioctl to get files' placement on
> NTFS the same way they can do it on a number of other linux filesystems.
>
> The result is returned in FIGETBSZ units, which is equal to sector size
> in ntfs driver. An error value zero is returned for resident, compressed,
> encrypted files, and for holes in sparse files.
>
> Tested on drives with different sector sizes and different cluster sizes.
Thanks for your patch. It skirts/looks over some limitations so instead of applying your patch I have open sourced the bmap implementation from Tuxera NTFS driver (with permission) and have pushed it to the open source NTFS tree at git://git.kernel.org/pub/scm/linux/kernel/git/aia21/ntfs.git.
I have sent a pull request to Linus so he will hopefully merge it into mainline.
You can compare it to yours but most important points are that you do not forbid non-data attributes and you completely ignore initialized size which leads to exposing stale on-disk data on read and to losing the written data on writes which is obviously bad. You also don't verify that the value to be returned isn't too large to fit in the return value size so you can potentially return truncated values on 32-bit kernels with 32-bit sector_t on a large volume where the sector number would need 64-bits which would then lead to disk corruption as the user would read/write random sectors on disk instead of the correct ones.
Best regards,
Anton
> Cc: Anton Altaparmakov <aia21@xxxxxxxxxx>
> Signed-off-by: Sergei Antonov <saproj@xxxxxxxxx>
> ---
> fs/ntfs/aops.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/fs/ntfs/aops.c b/fs/ntfs/aops.c
> index d267ea6..3972b6b 100644
> --- a/fs/ntfs/aops.c
> +++ b/fs/ntfs/aops.c
> @@ -1538,6 +1538,36 @@ err_out:
>
> #endif /* NTFS_RW */
>
> +static sector_t ntfs_bmap(struct address_space *mapping, sector_t block)
> +{
> + struct inode *vi = mapping->host;
> + ntfs_inode *ni = NTFS_I(vi);
> + ntfs_volume *vol = ni->vol;
> + const unsigned clust2sect_bits = vol->cluster_size_bits -
> + vol->sector_size_bits;
> + const sector_t clust2sect_mask = (1 << clust2sect_bits) - 1;
> + LCN lcn;
> +
> + BUG_ON(S_ISDIR(vi->i_mode));
> + ntfs_debug("Requested LCN for block %llu.", (unsigned long long)block);
> + if (!NInoNonResident(ni) || NInoCompressed(ni) || NInoEncrypted(ni))
> + return 0;
> +
> + down_read(&ni->runlist.lock);
> + lcn = ntfs_attr_vcn_to_lcn_nolock(ni, block >> clust2sect_bits, false);
> + up_read(&ni->runlist.lock);
> +
> + if (lcn < 0) {
> + if (lcn == LCN_HOLE)
> + ntfs_warning(vol->sb, "Cannot get LCN for a hole.");
> + else
> + ntfs_error(vol->sb, "Error getting LCN: %lld.", lcn);
> + return 0;
> + }
> +
> + return (lcn << clust2sect_bits) + (block & clust2sect_mask);
> +}
> +
> /**
> * ntfs_aops - general address space operations for inodes and attributes
> */
> @@ -1546,6 +1576,7 @@ const struct address_space_operations ntfs_aops = {
> #ifdef NTFS_RW
> .writepage = ntfs_writepage, /* Write dirty page to disk. */
> #endif /* NTFS_RW */
> + .bmap = ntfs_bmap, /* Map file offset to volume. */
> .migratepage = buffer_migrate_page, /* Move a page cache page from
> one physical page to an
> other. */
--
Anton Altaparmakov <anton at tuxera.com> (replace at with @)
Senior Kernel Engineer, Tuxera Inc., http://www.tuxera.com/
Linux NTFS maintainer
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
