Linux CIFS filesystem development
 help / color / mirror / Atom feed
* cifs hardlink misbehaviour in generic/002?
@ 2023-12-01 16:15 David Howells
  2023-12-01 17:29 ` Paulo Alcantara
  2023-12-02  0:05 ` Namjae Jeon
  0 siblings, 2 replies; 7+ messages in thread
From: David Howells @ 2023-12-01 16:15 UTC (permalink / raw)
  To: Steve French; +Cc: dhowells, ronniesahlberg, aaptel, linux-cifs

[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]


Hi Steve,

I've seeing some weird behaviour in the upstream Linux cifs filesystem that's
causing generic/002 to fail.  The test case makes a file and creates a number
of hardlinks to it, then deletes those hardlinks in reverse order, checking
the link count on the original file each time it removes one.

However, I'm seeing the original file disappear when the most recent link is
removed, leading to the check for the link count to fail due to $x evaluating
blank and the '[' program complaining about syntax.

I've distilled the testcase down to a small shell script:

	#!/bin/sh
	TEST_DIR=/xfstest.test/tmp/
	top=3
	if [ ! -d $TEST_DIR ]
	then
	    mkdir $TEST_DIR || exit $?
	else
	    rm $TEST_DIR/foo.*
	fi
	touch $TEST_DIR/foo.1
	for ((l=2; l<=$top; l++))
	do
	    ln -v $TEST_DIR/foo.1 $TEST_DIR/foo.$l
	done
	ls $TEST_DIR
	for ((l=$top; l>=1; l--))
	do
	    if [ ! -f $TEST_DIR/foo.1 ]
	    then
		echo "Arrgh, foo.1 is missing!"
		ls $TEST_DIR
		exit 1
	    fi
	    x=`stat -c %h $TEST_DIR/foo.1`
	    if [ "$l" -ne $x ]
	    then
		echo "Arrgh, incorrect link count"
		$here/src/lstat64 $TEST_DIR/foo.1
		status=1
	    fi
	    rm -v -f $TEST_DIR/foo.$l
	done

that I'm running on a Linux v6.6 ksmbd share mounted with:

	mount //192.168.6.1/test /xfstest.test -o user=...,pass=...,noperm,vers=3.1.1,cache=loose

The server includes the following settings:

        server role = standalone server
        log level = 1
        security = user
        load printers = no
        #smb3 encryption = yes
        netbios name = SMBD
        server multi channel support = yes
        smb2 leases = yes

The output I see is:

	'/xfstest.test/tmp//foo.2' => '/xfstest.test/tmp//foo.1'
	'/xfstest.test/tmp//foo.3' => '/xfstest.test/tmp//foo.1'
	foo.1  foo.2  foo.3
	removed '/xfstest.test/tmp//foo.3'
	Arrgh, foo.1 is missing!
	foo.1  foo.2  foo.3

I've attached a wireshark trace of the script being run.

I see a lot of STATUS_DELETE_PENDING apparently being applied to foo.1.

David


[-- Attachment #2: pcap.gz --]
[-- Type: application/gzip, Size: 3760 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: cifs hardlink misbehaviour in generic/002?
  2023-12-01 16:15 cifs hardlink misbehaviour in generic/002? David Howells
@ 2023-12-01 17:29 ` Paulo Alcantara
  2023-12-01 17:42   ` David Howells
  2023-12-01 17:43   ` Jeremy Allison
  2023-12-02  0:05 ` Namjae Jeon
  1 sibling, 2 replies; 7+ messages in thread
From: Paulo Alcantara @ 2023-12-01 17:29 UTC (permalink / raw)
  To: David Howells, Steve French; +Cc: dhowells, ronniesahlberg, aaptel, linux-cifs

David Howells <dhowells@redhat.com> writes:

> I've seeing some weird behaviour in the upstream Linux cifs filesystem that's
> causing generic/002 to fail.  The test case makes a file and creates a number
> of hardlinks to it, then deletes those hardlinks in reverse order, checking
> the link count on the original file each time it removes one.

I could also reproduce it in ksmbd but not in Windows Server 2022 or
samba v4.19.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: cifs hardlink misbehaviour in generic/002?
  2023-12-01 17:29 ` Paulo Alcantara
@ 2023-12-01 17:42   ` David Howells
  2023-12-01 17:43   ` Jeremy Allison
  1 sibling, 0 replies; 7+ messages in thread
From: David Howells @ 2023-12-01 17:42 UTC (permalink / raw)
  To: Paulo Alcantara
  Cc: dhowells, Steve French, ronniesahlberg, aaptel, linux-cifs

Paulo Alcantara <pc@manguebit.com> wrote:

> David Howells <dhowells@redhat.com> writes:
> 
> > I've seeing some weird behaviour in the upstream Linux cifs filesystem
> > that's causing generic/002 to fail.  The test case makes a file and
> > creates a number of hardlinks to it, then deletes those hardlinks in
> > reverse order, checking the link count on the original file each time it
> > removes one.
> 
> I could also reproduce it in ksmbd but not in Windows Server 2022 or
> samba v4.19.

Yeah - works for me with samba also; just not with ksmbd.

David


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: cifs hardlink misbehaviour in generic/002?
  2023-12-01 17:29 ` Paulo Alcantara
  2023-12-01 17:42   ` David Howells
@ 2023-12-01 17:43   ` Jeremy Allison
  2023-12-01 17:54     ` Paulo Alcantara
  1 sibling, 1 reply; 7+ messages in thread
From: Jeremy Allison @ 2023-12-01 17:43 UTC (permalink / raw)
  To: Paulo Alcantara
  Cc: David Howells, Steve French, ronniesahlberg, aaptel, linux-cifs

On Fri, Dec 01, 2023 at 02:29:41PM -0300, Paulo Alcantara wrote:
>David Howells <dhowells@redhat.com> writes:
>
>> I've seeing some weird behaviour in the upstream Linux cifs filesystem that's
>> causing generic/002 to fail.  The test case makes a file and creates a number
>> of hardlinks to it, then deletes those hardlinks in reverse order, checking
>> the link count on the original file each time it removes one.
>
>I could also reproduce it in ksmbd but not in Windows Server 2022 or
>samba v4.19.

Looks like a ksmbd bug where it's not checking for multiple
opens on close with DELETE_ON_CLOSE set, and just removing
the file.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: cifs hardlink misbehaviour in generic/002?
  2023-12-01 17:43   ` Jeremy Allison
@ 2023-12-01 17:54     ` Paulo Alcantara
  0 siblings, 0 replies; 7+ messages in thread
From: Paulo Alcantara @ 2023-12-01 17:54 UTC (permalink / raw)
  To: Jeremy Allison
  Cc: David Howells, Steve French, ronniesahlberg, aaptel, linux-cifs

Jeremy Allison <jra@samba.org> writes:

> Looks like a ksmbd bug where it's not checking for multiple
> opens on close with DELETE_ON_CLOSE set, and just removing
> the file.

Interesting.  Using 'nolease' mount option on the client make the issue
disappear.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: cifs hardlink misbehaviour in generic/002?
  2023-12-01 16:15 cifs hardlink misbehaviour in generic/002? David Howells
  2023-12-01 17:29 ` Paulo Alcantara
@ 2023-12-02  0:05 ` Namjae Jeon
  2023-12-05  9:39   ` David Howells
  1 sibling, 1 reply; 7+ messages in thread
From: Namjae Jeon @ 2023-12-02  0:05 UTC (permalink / raw)
  To: David Howells; +Cc: Steve French, ronniesahlberg, aaptel, linux-cifs

2023-12-02 1:15 GMT+09:00, David Howells <dhowells@redhat.com>:
>
> Hi Steve,
Hi David,

We have already sent the fixes on this week, So It has been in the
latest linus master.
Could you please check it on the latest master or after applying
commit 4274a9dc6aeb "ksmbd: separately allocate ci per dentry" ?

Thanks.
>
> I've seeing some weird behaviour in the upstream Linux cifs filesystem
> that's
> causing generic/002 to fail.  The test case makes a file and creates a
> number
> of hardlinks to it, then deletes those hardlinks in reverse order, checking
> the link count on the original file each time it removes one.
>
> However, I'm seeing the original file disappear when the most recent link
> is
> removed, leading to the check for the link count to fail due to $x
> evaluating
> blank and the '[' program complaining about syntax.
>
> I've distilled the testcase down to a small shell script:
>
> 	#!/bin/sh
> 	TEST_DIR=/xfstest.test/tmp/
> 	top=3
> 	if [ ! -d $TEST_DIR ]
> 	then
> 	    mkdir $TEST_DIR || exit $?
> 	else
> 	    rm $TEST_DIR/foo.*
> 	fi
> 	touch $TEST_DIR/foo.1
> 	for ((l=2; l<=$top; l++))
> 	do
> 	    ln -v $TEST_DIR/foo.1 $TEST_DIR/foo.$l
> 	done
> 	ls $TEST_DIR
> 	for ((l=$top; l>=1; l--))
> 	do
> 	    if [ ! -f $TEST_DIR/foo.1 ]
> 	    then
> 		echo "Arrgh, foo.1 is missing!"
> 		ls $TEST_DIR
> 		exit 1
> 	    fi
> 	    x=`stat -c %h $TEST_DIR/foo.1`
> 	    if [ "$l" -ne $x ]
> 	    then
> 		echo "Arrgh, incorrect link count"
> 		$here/src/lstat64 $TEST_DIR/foo.1
> 		status=1
> 	    fi
> 	    rm -v -f $TEST_DIR/foo.$l
> 	done
>
> that I'm running on a Linux v6.6 ksmbd share mounted with:
>
> 	mount //192.168.6.1/test /xfstest.test -o
> user=...,pass=...,noperm,vers=3.1.1,cache=loose
>
> The server includes the following settings:
>
>         server role = standalone server
>         log level = 1
>         security = user
>         load printers = no
>         #smb3 encryption = yes
>         netbios name = SMBD
>         server multi channel support = yes
>         smb2 leases = yes
>
> The output I see is:
>
> 	'/xfstest.test/tmp//foo.2' => '/xfstest.test/tmp//foo.1'
> 	'/xfstest.test/tmp//foo.3' => '/xfstest.test/tmp//foo.1'
> 	foo.1  foo.2  foo.3
> 	removed '/xfstest.test/tmp//foo.3'
> 	Arrgh, foo.1 is missing!
> 	foo.1  foo.2  foo.3
>
> I've attached a wireshark trace of the script being run.
>
> I see a lot of STATUS_DELETE_PENDING apparently being applied to foo.1.
>
> David
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: cifs hardlink misbehaviour in generic/002?
  2023-12-02  0:05 ` Namjae Jeon
@ 2023-12-05  9:39   ` David Howells
  0 siblings, 0 replies; 7+ messages in thread
From: David Howells @ 2023-12-05  9:39 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: dhowells, Steve French, ronniesahlberg, aaptel, linux-cifs

Namjae Jeon <linkinjeon@kernel.org> wrote:

> We have already sent the fixes on this week, So It has been in the
> latest linus master.
> Could you please check it on the latest master or after applying
> commit 4274a9dc6aeb "ksmbd: separately allocate ci per dentry" ?

Yep.  That fixes it.

David


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-12-05  9:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-01 16:15 cifs hardlink misbehaviour in generic/002? David Howells
2023-12-01 17:29 ` Paulo Alcantara
2023-12-01 17:42   ` David Howells
2023-12-01 17:43   ` Jeremy Allison
2023-12-01 17:54     ` Paulo Alcantara
2023-12-02  0:05 ` Namjae Jeon
2023-12-05  9:39   ` David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox