Re: [BUG?] There is a possibility that 'i_ino' overflows

[Tsutomu Itoh]

(2010/12/16 17:44), Tsutomu Itoh wrote:
> Hi,
> 
> In btrfs, inode number is increased each time a new file or directory
> is made.
> Therefore, if the making deletion of the file is repeated, value of
> 'i_ino' increases rapidly.
> 
> For example, inode number changes as follows. 
> 
>   $ touch foo
>   $ ls -i foo
>   266 foo
>   $ rm foo
>   $ touch bar
>   $ ls -i bar
>   267 bar
>   $
> 
> And then, length of 'i_ino' and 'objectid' is as follows on the x86
> system. 
> 
>   unsigned long i_ino == 32bits
>   u64 objectid== 64bits
> 
> Therefore, in the operation to substitute 'objectid' to 'i_ino',
> 'i_ino' overflows when 'objectid' 4294967296 is substituted to 'i_ino'. 
> Then, the file with inode number 0 is made.

I think that it is better to recycle inode number that became unused. 
And, at least, I think that it should make the filesystem not become an
abnormal condition. 

This patch is a patch that makes an error when inode number is bigger
than BTRFS_LAST_FREE_OBJECTID.


Signed-off-by: Tsutomu Itoh 
---
 inode.c |4 
 1 file changed, 4 insertions(+)

diff -urNp linux-2.6.37-rc6/fs/btrfs/inode.c 
linux-2.6.37-rc6.new/fs/btrfs/inode.c
--- linux-2.6.37-rc6/fs/btrfs/inode.c   2010-12-16 10:24:48.0 +0900
+++ linux-2.6.37-rc6.new/fs/btrfs/inode.c   2010-12-20 09:04:18.0 
+0900
@@ -4529,6 +4529,10 @@ static struct inode *btrfs_new_inode(str
 
inode_init_owner(inode, dir, mode);
inode->i_ino = objectid;
+   if (unlikely(inode->i_ino > (unsigned long)BTRFS_LAST_FREE_OBJECTID)) {
+   ret = -ENOSPC;
+   goto fail;
+   }
inode_set_bytes(inode, 0);
inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
inode_item = btrfs_item_ptr(path->nodes[0], path->slots[0],


> As a result, ls command has looped infinitely, and btrfsck detected the
> error.
> Please see below.
> 
>   $ uname -a
>   Linux luna 2.6.37-rc5 #1 SMP Thu Dec 9 13:02:41 JST 2010 i686 i686 i386 
> GNU/Linux
>   $ df -T /test1
>   FilesystemType   1K-blocks  Used Available Use% Mounted on
>   /dev/sdd14   btrfs 416256056   3717632   1% /test1
>   $ strace -FfTttx ls /test1
>   14:03:10.115440 execve("/bin/ls", ["ls", "/test1"], [/* 28 vars */]) = 0 
> <0.000181>
>   ...
>   ...
>   14:03:10.123431 stat("/test1", {st_mode=S_IFDIR|0777, st_size=8, ...}) = 0 
> <0.17>
>   14:03:10.123521 open("/test1", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 
> 3 <0.18>
>   14:03:10.123578 fcntl(3, F_GETFD)   = 0x1 (flags FD_CLOEXEC) <0.13>
>   14:03:10.123637 getdents(3, /* 3 entries */, 32768) = 72 <0.25>
>   14:03:10.123712 getdents(3, /* 1 entries */, 32768) = 24 <0.16>
>   14:03:10.123768 getdents(3, /* 1 entries */, 32768) = 24 <0.15>
>   14:03:10.123824 getdents(3, /* 1 entries */, 32768) = 24 <0.15>
>   14:03:10.123880 getdents(3, /* 1 entries */, 32768) = 24 <0.15>
>   14:03:10.123936 getdents(3, /* 1 entries */, 32768) = 24 <0.14>
>   14:03:10.123992 getdents(3, /* 1 entries */, 32768) = 24 <0.15>
>   14:03:10.124047 getdents(3, /* 1 entries */, 32768) = 24 <0.15>
>   14:03:10.124103 getdents(3, /* 1 entries */, 32768) = 24 <0.15>
>   14:03:10.124261 getdents(3, /* 1 entries */, 32768) = 24 <0.16>
>   14:03:10.124320 getdents(3, /* 1 entries */, 32768) = 24 <0.15>
>   14:03:10.124381 getdents(3, /* 1 entries */, 32768) = 24 <0.15>
>   14:03:10.124437 getdents(3, /* 1 entries */, 32768) = 24 <0.14>
>   14:03:10.124493 getdents(3, /* 1 entries */, 32768) = 24 <0.14>
>   14:03:10.124549 getdents(3, /* 1 entries */, 32768) = 24 <0.14>
>   14:03:10.124605 getdents(3, /* 1 entries */, 32768) = 24 <0.14>
>   14:03:10.124661 getdents(3, /* 1 entries */, 32768) = 24 <0.14>
>   14:03:10.124717 getdents(3, /* 1 entries */, 32768) = 24 <0.14>
>   14:03:10.124773 getdents(3, /* 1 entries */, 32768) = 24 <0.14>
>   14:03:10.124840 getdents(3, /* 1 entries */, 32768) = 24 <0.14>
>   14:03:10.124896 getdents(3, /* 1 entries */, 32768) = 24 <0.14>
>   ...
>   ...
>   $ cd /test1
>   $ ls -i A123
>   ls: cannot access A123: No such file or directory
>   $ touch 
>   $ ls -i 
>   1 
>   $
> 
>   # umount /test1
>   # btrfsck /dev/sdd14
>   root 5 inode 0 errors 2001
>   unresolved ref dir 256 index 4294967041 namelen 4 name A123 
> filetype 1 error 4
>   root 5 inode 1 errors 2001
>   unresolved ref dir 256 index 4294967042 namelen 4 name  
> filetype 1 error 4
>   root 5 inode 4294967296 errors 2000
>   unresolved ref dir 256 index 4294967041 namelen 4 name A123 
> filetype 0 error 3
>   root 5 inode 4294967297 errors 2000
>   unresolved ref dir 256 index 4294967042 namelen 4 name  
> filetype 0 error 3
>   found 28672 bytes used err is 1
>   total csum bytes: 0
>   total tree bytes: 28672
>   total fs tree bytes: 8192
>   btree space waste bytes: 23191
>   file data blocks allocate

[PATCH] Improve error handling in filesystem df

[Ben Gamari]

The return values of ioctl weren't being printed to stderr on failure,
causing the command to silently fail, resulting in a very confused
user.

Signed-off-by: Ben Gamari 
---
 btrfs_cmds.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/btrfs_cmds.c b/btrfs_cmds.c
index 8031c58..45da2bd 100644
--- a/btrfs_cmds.c
+++ b/btrfs_cmds.c
@@ -857,6 +857,7 @@ int do_df_filesystem(int nargs, char **argv)
 
ret = ioctl(fd, BTRFS_IOC_SPACE_INFO, sargs);
if (ret) {
+   fprintf(stderr, "ERROR: can't query '%s' for free space 
(%s)\n", path, strerror(-ret));
free(sargs);
return ret;
}
@@ -875,6 +876,7 @@ int do_df_filesystem(int nargs, char **argv)
 
ret = ioctl(fd, BTRFS_IOC_SPACE_INFO, sargs);
if (ret) {
+   fprintf(stderr, "ERROR: can't query '%s' for free space 
(%s)\n", path, strerror(-ret));
free(sargs);
return ret;
}
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[BUG] Calling BTRFS_IOC_TREE_SEARCH for a non existant key

[Goffredo Baroncelli]

Hi all,

I discovered that when I call the BTRFS_IOC_TREE_SEARCH ioctl for a not 
existent key, the ioctl requires a lot of time to be completed.

I tracks down the problem in the function search_ioctl(). It seems that the 
function btrfs_search_forward() doesn't check if key is greater than max_key. 

file ioctl.c
   
   988  static noinline int search_ioctl(struct inode *inode,
   989   struct btrfs_ioctl_search_args *args)
[...]
  1025  while(1) {
  1026  ret = btrfs_search_forward(root, &key, &max_key, path, 
0,
  1027 sk->min_transid);
  1028  if (ret != 0) {
  1029  if (ret > 0)
  1030  ret = 0;
  1031  goto err;
  1032  }
  1033  ret = copy_to_sk(root, path, &key, sk, args->buf,
  1034   &sk_offset, &num_found);
  1035  btrfs_release_path(root, path);
  1036  if (ret || num_found >= sk->nr_items)
  1037  break;
   
  1038  }

The function btrfs_search_forward() doesn't check the key (or min_key) against 
max_key when the parameter cache_only == 0.

from ctree.c

  3624  int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key 
*min_key,
  3625   struct btrfs_key *max_key,
  3626   struct btrfs_path *path, int cache_only,
  3627   u64 min_trans)
  3628  {
[..]
  3678  if (!cache_only)
  3679  break;
   
  3680  if (max_key) {
  3681  btrfs_node_key(cur, &disk_key, slot);
  3682  if (comp_keys(&disk_key, max_key) >= 
0) {
  3683  ret = 1;
  3684  goto out;
  3685  }
  3686  }
   


So in the function search_ioctl the search is not completed until the "args" 
buffer is filled or the function copy_to_sk detects that key is greater than 
sk->max_*. 

However the check performed by the function copy_to_sk is not always 
sufficient to detect if a key is greater than sk->max_*. For example if 
key.objectid > sk->max_objectid but key.offset is < sk->max_offset, the key is 
supposed to be valid.

   913  static noinline int copy_to_sk(struct btrfs_root *root,
[...]
   970  }
   971  advance_key:
   972  ret = 0;
   973  if (key->offset < (u64)-1 && key->offset < sk->max_offset)
   974  key->offset++;
   975  else if (key->type < (u8)-1 && key->type < sk->max_type) {
   976  key->offset = 0;
   977  key->type++;
   978  } else if (key->objectid < (u64)-1 && key->objectid < sk-
>max_objectid) {
   979  key->offset = 0;
   980  key->type = 0;
   981  key->objectid++;
   982  } else
   983  ret = 1;
   984  overflow:
   985  *num_found += found;
   986  return ret;
   987  }
  
To solve this problem it should be sufficient a key check before the call of 
btrfs_search_forward(). 
But I don't know if this is a workaround or the root cause is inside the 
function btrfs_search_forward() that doesn't check the key against max_key if 
cached_only is 0.


Regards
G.Baroncelli

-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) 
Key fingerprint = 4769 7E51 5293 D36C 814E  C054 BF04 F161 3DC5 0512
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html