Btrfs devel - Dec 2009 - BUG: Link from sub volume, then remove the subvolume -> wrong link

Hi all

when I tried to reproduce the bug highlighted by Andrew, I discovered another 
bug. When I link a file from a subvolume to another, then remove the subvolume 
I got a wrong link: in my test an *hard* link becomes a *soft* link !!!!

Steps to reproduce the bug:

	root@venice:/tmp/test# sudo btrfsctl -S subvol2 .
	operation complete
	Btrfs Btrfs v0.19
	root@venice:/tmp/test# echo 123 >>subvol2/myfile
	root@venice:/tmp/test# ln subvol2/myfile .
	root@venice:/tmp/test# ls -l . subvol2/
	.:
	total 4
	-rw-r--r-- 2 root root  4 2009-12-11 20:40 myfile
	drwx------ 1 root root 12 2009-12-11 20:40 subvol2

	subvol2/:
	total 4
	-rw-r--r-- 2 root root 4 2009-12-11 20:40 myfile
	root@venice:/tmp/test# btrfsctl -D subvol2 .
	operation complete
	Btrfs Btrfs v0.19
	root@venice:/tmp/test# ls -l
	total 0
	lrwxrwxrwx 1 root root 3 2009-11-22 10:56 myfile -> lib

Now myfile is a soft link instead to a hard link!!!! Moreover the soft link is 
to an incorrect target ( 1) lib doesn''t exist, 2) in any case is not
the right
target).


BR
Goffredo
-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo)
<kreijackATinwind.it>
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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

 I don''t know how a hard link becomes to a soft link, hard link
across subvolumes should not allowed in btrfs.

 Hard link contains target inode number but not target tree.
So, if we can create such hard link normally, it points to
a file which has same inode number in same subvolume.
 As for this problem, Christian Parpart posted a patch, and
I rebased it.

Regards,
taruisi

(2009/12/12 4:54), Goffredo Baroncelli wrote:
> Hi all
> 
> when I tried to reproduce the bug highlighted by Andrew, I discovered
another
> bug. When I link a file from a subvolume to another, then remove the
subvolume
> I got a wrong link: in my test an *hard* link becomes a *soft* link !!!!
> 
> Steps to reproduce the bug:
> 
> 	root@venice:/tmp/test# sudo btrfsctl -S subvol2 .
> 	operation complete
> 	Btrfs Btrfs v0.19
> 	root@venice:/tmp/test# echo 123 >>subvol2/myfile
> 	root@venice:/tmp/test# ln subvol2/myfile .
> 	root@venice:/tmp/test# ls -l . subvol2/
> 	.:
> 	total 4
> 	-rw-r--r-- 2 root root  4 2009-12-11 20:40 myfile
> 	drwx------ 1 root root 12 2009-12-11 20:40 subvol2
> 
> 	subvol2/:
> 	total 4
> 	-rw-r--r-- 2 root root 4 2009-12-11 20:40 myfile
> 	root@venice:/tmp/test# btrfsctl -D subvol2 .
> 	operation complete
> 	Btrfs Btrfs v0.19
> 	root@venice:/tmp/test# ls -l
> 	total 0
> 	lrwxrwxrwx 1 root root 3 2009-11-22 10:56 myfile -> lib
> 
> Now myfile is a soft link instead to a hard link!!!! Moreover the soft link
is
> to an incorrect target ( 1) lib doesn''t exist, 2) in any case is
not the right
> target).
> 
> 
> BR
> Goffredo

-- 
taruisi

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

On Saturday 12 December 2009, TARUISI Hiroaki wrote:
> Hi,
> 
>  I don''t know how a hard link becomes to a soft link, hard link
> across subvolumes should not allowed in btrfs.
I try an explanation below
> 
>  Hard link contains target inode number but not target tree.
> So, if we can create such hard link normally, it points to
> a file which has same inode number in same subvolume.
I can add further details. The next boot my machine was un-bootable. The 
reasons was that the *soft link* /lib64 (which pointed to /lib) was wrong.

My explanation of the fact is:
- In my root there was the link "/lib64 -> lib"; the file /lib64
had inode
number = 257.
- I created a subvolume for my test. Under this subvolume I created a file 
(called "myfile") which (casually) had inode number = 257.
- I hard linked the  file "myfile" (which inode number 257) in my root
volume.
- Unfortunately the inode number of the linked file, existed already and was 
the one of the /lib64 link.
- When I removed the subvolume used for my test, the two file (/lib64 and the 
linked target) exchanged its content and the hard-link becames a soft link and 
vice-versa.

The final results was:
- an unbootable machine
- three files with the same inode (and with a ref count equal to two !!)

On the basis of the facts above, I suggest to Chris to raise the importance of 
the patch of TARUISI Hiroaki (see email [PATCH] Deny sys_link across 
subvolumes 12/11/2009).

BR
Goffredo



-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo)
<kreijackATinwind.it>
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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

TARUISI Hiroaki wrote:
> Hi,
> 
>  I don''t know how a hard link becomes to a soft link, hard link
> across subvolumes should not allowed in btrfs.
> 
>  Hard link contains target inode number but not target tree.
> So, if we can create such hard link normally, it points to
> a file which has same inode number in same subvolume.
>  As for this problem, Christian Parpart posted a patch, and
> I rebased it.
Can you post a link?  I''d like to test it, and it should probably go 
into -stable (since it fixes an oops that can be triggered without 
privileges).

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

Hi,

We can see the patch in ML archive or at ''Patchwork'' site.

  http://patchwork.kernel.org/patch/59519/

Thanks,
taruisi

(2009/12/13 3:39), Andy Lutomirski wrote:
> TARUISI Hiroaki wrote:
>> Hi,
>>
>>  I don''t know how a hard link becomes to a soft link, hard
link
>> across subvolumes should not allowed in btrfs.
>>
>>  Hard link contains target inode number but not target tree.
>> So, if we can create such hard link normally, it points to
>> a file which has same inode number in same subvolume.
>>  As for this problem, Christian Parpart posted a patch, and
>> I rebased it.
> 
> Can you post a link?  I''d like to test it, and it should probably
go
> into -stable (since it fixes an oops that can be triggered without
> privileges).
> 
> Thanks,
> Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[cc: Chris -- This, or something like it, should probably go to
stable, but it needs to make it to upstream somewhere first.]

On Sun, Dec 13, 2009 at 6:42 PM, TARUISI Hiroaki
<taruishi.hiroak@jp.fujitsu.com> wrote:
> Hi,
>
> We can see the patch in ML archive or at ''Patchwork''
site.
>
>  http://patchwork.kernel.org/patch/59519/
That fixes the bug, but shouldn''t the error be EXDEV?  It has nothing
to do with permissions.

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

Hi,

I think message:''Operation not permitted'' is more
proper for this problem than ''Invalid cross-device
link'' simply because this link is not cross-device
link.

Of course, this operation is prohibited not by security
policy but by inner limitation of btrfs, this usage of
EPERM may be a kind of abuses...

Regards,
taruisi

(2009/12/15 3:43), Andrew Lutomirski wrote:
> [cc: Chris -- This, or something like it, should probably go to
> stable, but it needs to make it to upstream somewhere first.]
> 
> On Sun, Dec 13, 2009 at 6:42 PM, TARUISI Hiroaki
> <taruishi.hiroak@jp.fujitsu.com> wrote:
>> Hi,
>>
>> We can see the patch in ML archive or at ''Patchwork''
site.
>>
>>  http://patchwork.kernel.org/patch/59519/
> 
> That fixes the bug, but shouldn''t the error be EXDEV?  It has
nothing
> to do with permissions.
> 
> --Andy
> --
> To unsubscribe from this list: send the line "unsubscribe
linux-btrfs" 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-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

TARUISI Hiroaki wrote:
> Hi,
> 
> I think message:''Operation not permitted'' is more
> proper for this problem than ''Invalid cross-device
> link'' simply because this link is not cross-device
> link.
> 
> Of course, this operation is prohibited not by security
> policy but by inner limitation of btrfs, this usage of
> EPERM may be a kind of abuses...
> 
EOPNOTSUPP?  EINVAL?

Seriously, though, something should go in.  Chris, any opinion?

--Andy
> Regards,
> taruisi
> 
> (2009/12/15 3:43), Andrew Lutomirski wrote:
>> [cc: Chris -- This, or something like it, should probably go to
>> stable, but it needs to make it to upstream somewhere first.]
>>
>> On Sun, Dec 13, 2009 at 6:42 PM, TARUISI Hiroaki
>> <taruishi.hiroak@jp.fujitsu.com> wrote:
>>> Hi,
>>>
>>> We can see the patch in ML archive or at
''Patchwork'' site.
>>>
>>>  http://patchwork.kernel.org/patch/59519/
>> That fixes the bug, but shouldn''t the error be EXDEV?  It has
nothing
>> to do with permissions.
>>
>> --Andy
>> --
>> To unsubscribe from this list: send the line "unsubscribe
linux-btrfs" 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-btrfs" 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-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

how about supporting the operation, as a feature, and allowing a file
to be in two different subvolumes at the same time? Would that make
the link semantics too confusing?

-- 
Is it the time when there isn''t time to discuss but there is time to
act yet?
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Supporting it may be not so confusing, but it may need
a little format change to record tree and inode number.
I think it''s not so acceptable.

Regards,
taruisi

(2009/12/16 2:31), David Nicol wrote:
> how about supporting the operation, as a feature, and allowing a file
> to be in two different subvolumes at the same time? Would that make
> the link semantics too confusing?
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html