[v4] xfs: fix the size of xfs_mode_to_ftype table
diff mbox

Message ID 1482440841-10819-1-git-send-email-amir73il@gmail.com
State New
Headers show

Commit Message

Amir Goldstein Dec. 22, 2016, 9:07 p.m. UTC
Fix the size of the xfs_mode_to_ftype conversion table,
which was too small to handle a malformed on-disk value
of mode=S_IFMT.

Use a convenience macros S_DT(mode) to convert from
mode to dirent file type and change the name of the table
to xfs_dtype_to_ftype to correctly describe its index values.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_dir2.c | 17 ++++++++---------
 fs/xfs/libxfs/xfs_dir2.h |  4 +++-
 fs/xfs/xfs_iops.c        |  2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

Darrick,

So my holiday season cleanup is now down to this one patch.
I pulled back the common code and added the needed macros in
xfs code, so this can be safely applied to xfsprogs as well.
I will send a patch to xfsprogs later.

Tested with generic/396 with -n ftype=0|1.

Amir.

v4:
- independent fix patch for xfs

v3:
- resort to simpler cleanup with macros DT_MAX and S_DT()
- mention the minor bug fix in commit message

v2:
- add private conversion from common to on-disk values

v1:
- use common conversion functions to get on-disk values

Comments

Darrick J. Wong Dec. 23, 2016, 9:01 p.m. UTC | #1
On Thu, Dec 22, 2016 at 11:07:21PM +0200, Amir Goldstein wrote:
> Fix the size of the xfs_mode_to_ftype conversion table,
> which was too small to handle a malformed on-disk value
> of mode=S_IFMT.
> 
> Use a convenience macros S_DT(mode) to convert from
> mode to dirent file type and change the name of the table
> to xfs_dtype_to_ftype to correctly describe its index values.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_dir2.c | 17 ++++++++---------
>  fs/xfs/libxfs/xfs_dir2.h |  4 +++-
>  fs/xfs/xfs_iops.c        |  2 +-
>  3 files changed, 12 insertions(+), 11 deletions(-)
> 
> Darrick,
> 
> So my holiday season cleanup is now down to this one patch.
> I pulled back the common code and added the needed macros in
> xfs code, so this can be safely applied to xfsprogs as well.
> I will send a patch to xfsprogs later.
> 
> Tested with generic/396 with -n ftype=0|1.

Hm... the 396 test looks ok, but I think we'd need a fs-specific
testcase to go write a corrupt i_mode = S_IFMT filesystem to try
to trigger the verifiers, right?

> Amir.
> 
> v4:
> - independent fix patch for xfs
> 
> v3:
> - resort to simpler cleanup with macros DT_MAX and S_DT()
> - mention the minor bug fix in commit message
> 
> v2:
> - add private conversion from common to on-disk values
> 
> v1:
> - use common conversion functions to get on-disk values
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index c58d72c..539f498 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -41,15 +41,14 @@ struct xfs_name xfs_name_dotdot = { (unsigned char *)"..", 2, XFS_DIR3_FT_DIR };
>   * for file type specification. This will be propagated into the directory
>   * structure if appropriate for the given operation and filesystem config.
>   */
> -const unsigned char xfs_mode_to_ftype[S_IFMT >> S_SHIFT] = {
> -	[0]			= XFS_DIR3_FT_UNKNOWN,
> -	[S_IFREG >> S_SHIFT]    = XFS_DIR3_FT_REG_FILE,
> -	[S_IFDIR >> S_SHIFT]    = XFS_DIR3_FT_DIR,
> -	[S_IFCHR >> S_SHIFT]    = XFS_DIR3_FT_CHRDEV,
> -	[S_IFBLK >> S_SHIFT]    = XFS_DIR3_FT_BLKDEV,
> -	[S_IFIFO >> S_SHIFT]    = XFS_DIR3_FT_FIFO,
> -	[S_IFSOCK >> S_SHIFT]   = XFS_DIR3_FT_SOCK,
> -	[S_IFLNK >> S_SHIFT]    = XFS_DIR3_FT_SYMLINK,
> +const unsigned char xfs_dtype_to_ftype[S_DT_MAX] = {
> +	[DT_REG]    = XFS_DIR3_FT_REG_FILE,
> +	[DT_DIR]    = XFS_DIR3_FT_DIR,
> +	[DT_CHR]    = XFS_DIR3_FT_CHRDEV,
> +	[DT_BLK]    = XFS_DIR3_FT_BLKDEV,
> +	[DT_FIFO]   = XFS_DIR3_FT_FIFO,
> +	[DT_SOCK]   = XFS_DIR3_FT_SOCK,
> +	[DT_LNK]    = XFS_DIR3_FT_SYMLINK,
>  };
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
> index 0197590..a069c3e 100644
> --- a/fs/xfs/libxfs/xfs_dir2.h
> +++ b/fs/xfs/libxfs/xfs_dir2.h
> @@ -35,7 +35,9 @@ extern struct xfs_name	xfs_name_dotdot;
>   * directory filetype conversion tables.
>   */
>  #define S_SHIFT 12
> -extern const unsigned char xfs_mode_to_ftype[];
> +#define S_DT(mode)     (((mode) & S_IFMT) >> S_SHIFT)
> +#define S_DT_MAX       (S_DT(S_IFMT) + 1)
> +extern const unsigned char xfs_dtype_to_ftype[];
>  
>  /*
>   * directory operations vector for encode/decode routines
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 308bebb..d2da9ca 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -103,7 +103,7 @@ xfs_dentry_to_name(
>  {
>  	namep->name = dentry->d_name.name;
>  	namep->len = dentry->d_name.len;
> -	namep->type = xfs_mode_to_ftype[(mode & S_IFMT) >> S_SHIFT];
> +	namep->type = xfs_dtype_to_ftype[S_DT(mode)];

So I think your concern here is that we could read a inode in from disk
that has an invalid i_mode such that we'd overflow xfs_mode_to_ftype
when trying to set the dirent ftype while linking the inode into a
directory, correct?

In that case, xfs_iread calls xfs_iformat_fork as part of pulling the
inode in off disk, and xfs_iformat_fork validates that the S_IFMT part
of i_mode actually corresponds to a known inode type and sends back 
-EFSCORRUPTED if not.

I think that checking suffices to handle this.  I prefer that we look
for invalid on-disk metadata and prevent it from being loaded into
memory at all, rather than try to catch all the problems that happen as
a result of insufficient validation.

--D

>  }
>  
>  STATIC void
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amir Goldstein Dec. 24, 2016, 7:31 a.m. UTC | #2
On Fri, Dec 23, 2016 at 11:01 PM, Darrick J. Wong
<darrick.wong@oracle.com> wrote:
> On Thu, Dec 22, 2016 at 11:07:21PM +0200, Amir Goldstein wrote:
>> Fix the size of the xfs_mode_to_ftype conversion table,
>> which was too small to handle a malformed on-disk value
>> of mode=S_IFMT.
>>
>> Use a convenience macros S_DT(mode) to convert from
>> mode to dirent file type and change the name of the table
>> to xfs_dtype_to_ftype to correctly describe its index values.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  fs/xfs/libxfs/xfs_dir2.c | 17 ++++++++---------
>>  fs/xfs/libxfs/xfs_dir2.h |  4 +++-
>>  fs/xfs/xfs_iops.c        |  2 +-
>>  3 files changed, 12 insertions(+), 11 deletions(-)
>>
>> Darrick,
>>
>> So my holiday season cleanup is now down to this one patch.
>> I pulled back the common code and added the needed macros in
>> xfs code, so this can be safely applied to xfsprogs as well.
>> I will send a patch to xfsprogs later.
>>
>> Tested with generic/396 with -n ftype=0|1.
>
> Hm... the 396 test looks ok, but I think we'd need a fs-specific
> testcase to go write a corrupt i_mode = S_IFMT filesystem to try
> to trigger the verifiers, right?
>

Right. I will write that testcase.

>> Amir.
>>
>> v4:
>> - independent fix patch for xfs
>>
>> v3:
>> - resort to simpler cleanup with macros DT_MAX and S_DT()
>> - mention the minor bug fix in commit message
>>
>> v2:
>> - add private conversion from common to on-disk values
>>
>> v1:
>> - use common conversion functions to get on-disk values
>>
>> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
>> index c58d72c..539f498 100644
>> --- a/fs/xfs/libxfs/xfs_dir2.c
>> +++ b/fs/xfs/libxfs/xfs_dir2.c
>> @@ -41,15 +41,14 @@ struct xfs_name xfs_name_dotdot = { (unsigned char *)"..", 2, XFS_DIR3_FT_DIR };
>>   * for file type specification. This will be propagated into the directory
>>   * structure if appropriate for the given operation and filesystem config.
>>   */
>> -const unsigned char xfs_mode_to_ftype[S_IFMT >> S_SHIFT] = {
>> -     [0]                     = XFS_DIR3_FT_UNKNOWN,
>> -     [S_IFREG >> S_SHIFT]    = XFS_DIR3_FT_REG_FILE,
>> -     [S_IFDIR >> S_SHIFT]    = XFS_DIR3_FT_DIR,
>> -     [S_IFCHR >> S_SHIFT]    = XFS_DIR3_FT_CHRDEV,
>> -     [S_IFBLK >> S_SHIFT]    = XFS_DIR3_FT_BLKDEV,
>> -     [S_IFIFO >> S_SHIFT]    = XFS_DIR3_FT_FIFO,
>> -     [S_IFSOCK >> S_SHIFT]   = XFS_DIR3_FT_SOCK,
>> -     [S_IFLNK >> S_SHIFT]    = XFS_DIR3_FT_SYMLINK,
>> +const unsigned char xfs_dtype_to_ftype[S_DT_MAX] = {
>> +     [DT_REG]    = XFS_DIR3_FT_REG_FILE,
>> +     [DT_DIR]    = XFS_DIR3_FT_DIR,
>> +     [DT_CHR]    = XFS_DIR3_FT_CHRDEV,
>> +     [DT_BLK]    = XFS_DIR3_FT_BLKDEV,
>> +     [DT_FIFO]   = XFS_DIR3_FT_FIFO,
>> +     [DT_SOCK]   = XFS_DIR3_FT_SOCK,
>> +     [DT_LNK]    = XFS_DIR3_FT_SYMLINK,
>>  };
>>
>>  /*
>> diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
>> index 0197590..a069c3e 100644
>> --- a/fs/xfs/libxfs/xfs_dir2.h
>> +++ b/fs/xfs/libxfs/xfs_dir2.h
>> @@ -35,7 +35,9 @@ extern struct xfs_name      xfs_name_dotdot;
>>   * directory filetype conversion tables.
>>   */
>>  #define S_SHIFT 12
>> -extern const unsigned char xfs_mode_to_ftype[];
>> +#define S_DT(mode)     (((mode) & S_IFMT) >> S_SHIFT)
>> +#define S_DT_MAX       (S_DT(S_IFMT) + 1)
>> +extern const unsigned char xfs_dtype_to_ftype[];
>>
>>  /*
>>   * directory operations vector for encode/decode routines
>> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
>> index 308bebb..d2da9ca 100644
>> --- a/fs/xfs/xfs_iops.c
>> +++ b/fs/xfs/xfs_iops.c
>> @@ -103,7 +103,7 @@ xfs_dentry_to_name(
>>  {
>>       namep->name = dentry->d_name.name;
>>       namep->len = dentry->d_name.len;
>> -     namep->type = xfs_mode_to_ftype[(mode & S_IFMT) >> S_SHIFT];
>> +     namep->type = xfs_dtype_to_ftype[S_DT(mode)];
>
> So I think your concern here is that we could read a inode in from disk
> that has an invalid i_mode such that we'd overflow xfs_mode_to_ftype
> when trying to set the dirent ftype while linking the inode into a
> directory, correct?
>
> In that case, xfs_iread calls xfs_iformat_fork as part of pulling the
> inode in off disk, and xfs_iformat_fork validates that the S_IFMT part
> of i_mode actually corresponds to a known inode type and sends back
> -EFSCORRUPTED if not.
>
> I think that checking suffices to handle this.  I prefer that we look
> for invalid on-disk metadata and prevent it from being loaded into
> memory at all, rather than try to catch all the problems that happen as
> a result of insufficient validation.
>

In the context of the justification that I provided in the commit message of
this patch, I agree with you. Like ext4, xfs also provides proper protection
against reading malformed i_mode data from disk (unlike ext2 and its clones).
So I shouldn't have included the malformed i_mode argument in my patch,
when I copied the commit message over from ext2 patch.

OTOH, is it really wise to keep the table at size 15 when fixing its size as
the patch suggests costs practically nothing? better safe then sorry.

And now for my real reasoning behind this patch.

From the point it time when xfs reflink is declared stable,
using it as underlying (host) fs for overlayfs containers, is going to become
a superior alternative to other local fs, because of:
 2ea9846 ovl: use vfs_clone_file_range() for copy up if possible

This head start paves the way to more xfs+overlayfs synergy.

For one such feature, I posted an early POC to linux-unionfs:
  ovl: use RENAME_DT_WHT to optimize ovl_dir_read_merged()
https://marc.info/?l=linux-unionfs&m=148242757001436&w=3

This requires fs specific support, for which I implemented a demo with xfs:
  xfs: support RENAME_VFS_DTYPE flag
https://marc.info/?l=linux-unionfs&m=148242757001433&w=3

I wanted to avoid cross posting to lists, but I probably should've CC'ed
you for context.

So now you understand my reasoning for fixing xfs_mode_to_ftype[]
to not assume anything about input mode.

Of course, this xfs_mode_to_ftype size fix can wait in by topic branch
until the future time when the said optimization will be considered and
after I propose the needed feature flag to xfs.

The reason I sent it out as is, detached from context and independent
of future developments is that I believe fixing the table size does have
merit on its own, certainly with nothing to loose.

FYI, at this moment, neither vfs_mknod() nor xfs_vn_mknod() sanitize
the mode argument. I know that overlayfs does not abuse this, but I did
not check if the 2 other call sites that call vfs_mknod (ecryptfs and nfsd)
sanitize mode.
That could be fixes of course in either vfs_mknod() and xfs_vn_mknod()
as well as audited in calling code.

Cheers,
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Brian Foster Dec. 24, 2016, 2:11 p.m. UTC | #3
On Thu, Dec 22, 2016 at 11:07:21PM +0200, Amir Goldstein wrote:
> Fix the size of the xfs_mode_to_ftype conversion table,
> which was too small to handle a malformed on-disk value
> of mode=S_IFMT.
> 
> Use a convenience macros S_DT(mode) to convert from
> mode to dirent file type and change the name of the table
> to xfs_dtype_to_ftype to correctly describe its index values.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---

A couple nits...

>  fs/xfs/libxfs/xfs_dir2.c | 17 ++++++++---------
>  fs/xfs/libxfs/xfs_dir2.h |  4 +++-
>  fs/xfs/xfs_iops.c        |  2 +-
>  3 files changed, 12 insertions(+), 11 deletions(-)
> 
> Darrick,
> 
> So my holiday season cleanup is now down to this one patch.
> I pulled back the common code and added the needed macros in
> xfs code, so this can be safely applied to xfsprogs as well.
> I will send a patch to xfsprogs later.
> 
> Tested with generic/396 with -n ftype=0|1.
> 
> Amir.
> 
> v4:
> - independent fix patch for xfs
> 
> v3:
> - resort to simpler cleanup with macros DT_MAX and S_DT()
> - mention the minor bug fix in commit message
> 
> v2:
> - add private conversion from common to on-disk values
> 
> v1:
> - use common conversion functions to get on-disk values
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index c58d72c..539f498 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -41,15 +41,14 @@ struct xfs_name xfs_name_dotdot = { (unsigned char *)"..", 2, XFS_DIR3_FT_DIR };
>   * for file type specification. This will be propagated into the directory
>   * structure if appropriate for the given operation and filesystem config.
>   */
> -const unsigned char xfs_mode_to_ftype[S_IFMT >> S_SHIFT] = {
> -	[0]			= XFS_DIR3_FT_UNKNOWN,
> -	[S_IFREG >> S_SHIFT]    = XFS_DIR3_FT_REG_FILE,
> -	[S_IFDIR >> S_SHIFT]    = XFS_DIR3_FT_DIR,
> -	[S_IFCHR >> S_SHIFT]    = XFS_DIR3_FT_CHRDEV,
> -	[S_IFBLK >> S_SHIFT]    = XFS_DIR3_FT_BLKDEV,
> -	[S_IFIFO >> S_SHIFT]    = XFS_DIR3_FT_FIFO,
> -	[S_IFSOCK >> S_SHIFT]   = XFS_DIR3_FT_SOCK,
> -	[S_IFLNK >> S_SHIFT]    = XFS_DIR3_FT_SYMLINK,
> +const unsigned char xfs_dtype_to_ftype[S_DT_MAX] = {

Please retain the use of FT_UNKNOWN.

> +	[DT_REG]    = XFS_DIR3_FT_REG_FILE,
> +	[DT_DIR]    = XFS_DIR3_FT_DIR,
> +	[DT_CHR]    = XFS_DIR3_FT_CHRDEV,
> +	[DT_BLK]    = XFS_DIR3_FT_BLKDEV,
> +	[DT_FIFO]   = XFS_DIR3_FT_FIFO,
> +	[DT_SOCK]   = XFS_DIR3_FT_SOCK,
> +	[DT_LNK]    = XFS_DIR3_FT_SYMLINK,
>  };
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
> index 0197590..a069c3e 100644
> --- a/fs/xfs/libxfs/xfs_dir2.h
> +++ b/fs/xfs/libxfs/xfs_dir2.h
> @@ -35,7 +35,9 @@ extern struct xfs_name	xfs_name_dotdot;
>   * directory filetype conversion tables.
>   */
>  #define S_SHIFT 12
> -extern const unsigned char xfs_mode_to_ftype[];
> +#define S_DT(mode)     (((mode) & S_IFMT) >> S_SHIFT)
> +#define S_DT_MAX       (S_DT(S_IFMT) + 1)

I think I'd prefer to see the '+ 1' in the xfs_dtype_to_ftype()
definition.

Those aside, this patch seems fine to me with the simple justification
that the array size doesn't match the set of index values we
limit/validate/scope mode against. AFAIU, this isn't a known bug and
applies to mode values either read from disk or passed in externally.
Darrick has pointed out that we already validate mode values on disk. I
assume we (the kernel) do the same thing for mode values that are passed
to XFS, but I don't see anything wrong with considering invalid input
sufficiently to prevent us from doing something stupid (going off the
end of the array and crashing or, perhaps worse, reading a valid ftype
value) due to potential external (or future) brokenness or corruption.

IOW, it's a landmine fixup in a currently non-reproducible error
condition bundled with a minor cleanup to avoid all of the ugly shifting
in the array initialization.

Brian

> +extern const unsigned char xfs_dtype_to_ftype[];
>  
>  /*
>   * directory operations vector for encode/decode routines
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 308bebb..d2da9ca 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -103,7 +103,7 @@ xfs_dentry_to_name(
>  {
>  	namep->name = dentry->d_name.name;
>  	namep->len = dentry->d_name.len;
> -	namep->type = xfs_mode_to_ftype[(mode & S_IFMT) >> S_SHIFT];
> +	namep->type = xfs_dtype_to_ftype[S_DT(mode)];
>  }
>  
>  STATIC void
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index c58d72c..539f498 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -41,15 +41,14 @@  struct xfs_name xfs_name_dotdot = { (unsigned char *)"..", 2, XFS_DIR3_FT_DIR };
  * for file type specification. This will be propagated into the directory
  * structure if appropriate for the given operation and filesystem config.
  */
-const unsigned char xfs_mode_to_ftype[S_IFMT >> S_SHIFT] = {
-	[0]			= XFS_DIR3_FT_UNKNOWN,
-	[S_IFREG >> S_SHIFT]    = XFS_DIR3_FT_REG_FILE,
-	[S_IFDIR >> S_SHIFT]    = XFS_DIR3_FT_DIR,
-	[S_IFCHR >> S_SHIFT]    = XFS_DIR3_FT_CHRDEV,
-	[S_IFBLK >> S_SHIFT]    = XFS_DIR3_FT_BLKDEV,
-	[S_IFIFO >> S_SHIFT]    = XFS_DIR3_FT_FIFO,
-	[S_IFSOCK >> S_SHIFT]   = XFS_DIR3_FT_SOCK,
-	[S_IFLNK >> S_SHIFT]    = XFS_DIR3_FT_SYMLINK,
+const unsigned char xfs_dtype_to_ftype[S_DT_MAX] = {
+	[DT_REG]    = XFS_DIR3_FT_REG_FILE,
+	[DT_DIR]    = XFS_DIR3_FT_DIR,
+	[DT_CHR]    = XFS_DIR3_FT_CHRDEV,
+	[DT_BLK]    = XFS_DIR3_FT_BLKDEV,
+	[DT_FIFO]   = XFS_DIR3_FT_FIFO,
+	[DT_SOCK]   = XFS_DIR3_FT_SOCK,
+	[DT_LNK]    = XFS_DIR3_FT_SYMLINK,
 };
 
 /*
diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
index 0197590..a069c3e 100644
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -35,7 +35,9 @@  extern struct xfs_name	xfs_name_dotdot;
  * directory filetype conversion tables.
  */
 #define S_SHIFT 12
-extern const unsigned char xfs_mode_to_ftype[];
+#define S_DT(mode)     (((mode) & S_IFMT) >> S_SHIFT)
+#define S_DT_MAX       (S_DT(S_IFMT) + 1)
+extern const unsigned char xfs_dtype_to_ftype[];
 
 /*
  * directory operations vector for encode/decode routines
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 308bebb..d2da9ca 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -103,7 +103,7 @@  xfs_dentry_to_name(
 {
 	namep->name = dentry->d_name.name;
 	namep->len = dentry->d_name.len;
-	namep->type = xfs_mode_to_ftype[(mode & S_IFMT) >> S_SHIFT];
+	namep->type = xfs_dtype_to_ftype[S_DT(mode)];
 }
 
 STATIC void