CIFS kernel module bug

CIFS kernel module bug

[Anton Altaparmakov]

Hi,

Looking at the current kernel (in Linus' repository on github) there is a silly logic bug in the cifs module in fs/cifs/cifsfs.c::cifs_llseek() there is this bit of code:

	/*
	 * origin == SEEK_END || SEEK_DATA || SEEK_HOLE => we must revalidate
	 * the cached file length
	 */
	if (origin != SEEK_SET || origin != SEEK_CUR) {

The logical or should be a logical and, i.e. this should be:

	if (origin != SEEK_SET && origin != SEEK_CUR) {

As the code is at present that line is ALWAYS true because origin is ALWAYS either != SEEK_SET or != SEEK_CUR as if it equals one it cannot equal the other and vice versa…

So at the moment it always does the revalidation instead of only for SEEK_END, SEEK_DATA, and SEEK_HOLE.

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/

Re: CIFS kernel module bug

[Jeff Layton]

On Fri, 30 Sep 2011 14:58:58 +0100
Anton Altaparmakov <aia21@cam.ac.uk> wrote:

> Hi,
> 
> Looking at the current kernel (in Linus' repository on github) there is a silly logic bug in the cifs module in fs/cifs/cifsfs.c::cifs_llseek() there is this bit of code:
> 
> 	/*
> 	 * origin == SEEK_END || SEEK_DATA || SEEK_HOLE => we must revalidate
> 	 * the cached file length
> 	 */
> 	if (origin != SEEK_SET || origin != SEEK_CUR) {
> 
> The logical or should be a logical and, i.e. this should be:
> 
> 	if (origin != SEEK_SET && origin != SEEK_CUR) {
> 
> As the code is at present that line is ALWAYS true because origin is ALWAYS either != SEEK_SET or != SEEK_CUR as if it equals one it cannot equal the other and vice versa…
> 
> So at the moment it always does the revalidation instead of only for SEEK_END, SEEK_DATA, and SEEK_HOLE.
> 
> Best regards,
> 
> 	Anton


Haha, good catch. Care to roll up a patch to fix that?

-- 
Jeff Layton <jlayton@samba.org>

Re: CIFS kernel module bug

[Steve French]

On Fri, Sep 30, 2011 at 1:04 PM, Jeff Layton <jlayton@samba.org> wrote:
> On Fri, 30 Sep 2011 14:58:58 +0100
> Anton Altaparmakov <aia21@cam.ac.uk> wrote:
>
>> Hi,
>>
>> Looking at the current kernel (in Linus' repository on github) there is a silly logic bug in the cifs module in fs/cifs/cifsfs.c::cifs_llseek() there is this bit of code:
>>
>>       /*
>>        * origin == SEEK_END || SEEK_DATA || SEEK_HOLE => we must revalidate
>>        * the cached file length
>>        */
>>       if (origin != SEEK_SET || origin != SEEK_CUR) {
>>
>> The logical or should be a logical and, i.e. this should be:
>>
>>       if (origin != SEEK_SET && origin != SEEK_CUR) {
>>
>> As the code is at present that line is ALWAYS true because origin is ALWAYS either != SEEK_SET or != SEEK_CUR as if it equals one it cannot equal the other and vice versa…
>>
>> So at the moment it always does the revalidation instead of only for SEEK_END, SEEK_DATA, and SEEK_HOLE.
>>
>> Best regards,
>>
>>       Anton
>
>
> Haha, good catch. Care to roll up a patch to fix that?
>
> --
> Jeff Layton <jlayton@samba.org>
>


Yes - obviously that code was wrong, fortunately not a disaster.
Thanks for pointing this out.  If you don't want to write up
the patch let us know and we will make the trivial fix.



-- 
Thanks,

Steve

Re: CIFS kernel module bug

[Anton Altaparmakov]

Hi,

On 30 Sep 2011, at 19:04, Jeff Layton wrote:
> On Fri, 30 Sep 2011 14:58:58 +0100 Anton Altaparmakov <aia21@cam.ac.uk> wrote:
> 
>> Hi,
>> 
>> Looking at the current kernel (in Linus' repository on github) there is a silly logic bug in the cifs module in fs/cifs/cifsfs.c::cifs_llseek() there is this bit of code:
>> 
>> 	/*
>> 	 * origin == SEEK_END || SEEK_DATA || SEEK_HOLE => we must revalidate
>> 	 * the cached file length
>> 	 */
>> 	if (origin != SEEK_SET || origin != SEEK_CUR) {
>> 
>> The logical or should be a logical and, i.e. this should be:
>> 
>> 	if (origin != SEEK_SET && origin != SEEK_CUR) {
>> 
>> As the code is at present that line is ALWAYS true because origin is ALWAYS either != SEEK_SET or != SEEK_CUR as if it equals one it cannot equal the other and vice versa…
>> 
>> So at the moment it always does the revalidation instead of only for SEEK_END, SEEK_DATA, and SEEK_HOLE.
> 
> Haha, good catch. Care to roll up a patch to fix that?


Yes, I am happy to do that.

Also a question for you: I am up to the neck in having to adapt the CIFS module as it doesn't work very well against the Novell Open Enterprise Server CIFS server (that is why I was looking at the CIFS code and noticed this logic bug in the first place).

A bit of background: the home directories for our (i.e. University of Cambridge Computing Service) Linux distribution "MCS Linux" which runs on about a thousand workstations and a handfull of remote access servers used to be on Netware and were moved to OES Linux recently and the ncpfs kernel module really fell over in a heap when running against the OES NCP server so we had to switch to CIFS in a hurry and whilst it does not fall over in a heap like ncpfs does and we now have working reconnects when the home directory server is rebooted (home directories magically start working again after the reboot which is brilliant!) many applications do not work.

I can only assume this is because the CIFS server is an abomination rather than that the CIFS modules is quite that badly broken…  Anyway, my question is: do you want patches that make the CIFS module work better with OES CIFS server or do you not care?

For example lots of applications require working hard links so I had to port our Virtual Hard Link implementation from NCPfs to CIFS to get hard links working which fixed a lot of applications but then we started hitting real bugs.  A few examples:

- llseek(SEEK_END) fails against the OES server with error -EBADF from the server because the SEEK_END causes file revalidation which ends up calling CIFSSMBQFileInfo() which then results in the -EBADF.  I wrote a patch to fall back to path based query via CIFSSMBQPathInfo() and if that fails via SMBQueryInformation() and now the llseek(SEEK_END) works thus applications like Seamonkey actually manage to launch rather than segfaulting.

- mkdir() returns -EACCES when the target exists.  Again this is coming from the OES server.  I wrote a patch that calls cifs_get_inode_info() when CIFSSMBMkDir() returns -EACCES and unix extensions are not enabled on the superblock and if the result is success I rewrite the return code to -EEXIST which fixes the mkdir issue and that fixes loads of Gnome related applications.

- The OES server behaves like NT4 in that it returns success from CIFSSMBSetPathInfo() but it actually ignores the call unless the file is open on the server.  But the OES server does not have CIFS_SES_NT4 in the session flags thus the work around in the CIFS module does not trigger and thus changing the access times or permissions does not work unless the file happens to be open which breaks applications like rsync.  I commented the CIFSSMBSetPathInfo() call completely in a patch so it falls back to the other code which makes it work for us.

And finally the actual question: Are you interested in any of these patches or at least are you interested in fixing these issues in the standard CIFS module in which case I can send you my patches or at least give you detailed descriptions of what is failing and how?

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/

Re: CIFS kernel module bug

[Steve French]

On Fri, Sep 30, 2011 at 4:30 PM, Anton Altaparmakov <aia21@cam.ac.uk> wrote:
> Hi,
>
> On 30 Sep 2011, at 19:04, Jeff Layton wrote:
>> On Fri, 30 Sep 2011 14:58:58 +0100 Anton Altaparmakov <aia21@cam.ac.uk> wrote:
>>
>>> Hi,
>>>
>>> Looking at the current kernel (in Linus' repository on github) there is a silly logic bug in the cifs module in fs/cifs/cifsfs.c::cifs_llseek() there is this bit of code:
>>>
>>>      /*
>>>       * origin == SEEK_END || SEEK_DATA || SEEK_HOLE => we must revalidate
>>>       * the cached file length
>>>       */
>>>      if (origin != SEEK_SET || origin != SEEK_CUR) {
>>>
>>> The logical or should be a logical and, i.e. this should be:
>>>
>>>      if (origin != SEEK_SET && origin != SEEK_CUR) {
>>>
>>> As the code is at present that line is ALWAYS true because origin is ALWAYS either != SEEK_SET or != SEEK_CUR as if it equals one it cannot equal the other and vice versa…
>>>
>>> So at the moment it always does the revalidation instead of only for SEEK_END, SEEK_DATA, and SEEK_HOLE.
>>
>> Haha, good catch. Care to roll up a patch to fix that?
>
>
> Yes, I am happy to do that.
>
> Also a question for you: I am up to the neck in having to adapt the CIFS module as it doesn't work very well against the Novell Open Enterprise Server CIFS server (that is why I was looking at the CIFS code and noticed this logic bug in the first place).
>
> A bit of background: the home directories for our (i.e. University of Cambridge Computing Service) Linux distribution "MCS Linux" which runs on about a thousand workstations and a handfull of remote access servers used to be on Netware and were moved to OES Linux recently and the ncpfs kernel module really fell over in a heap when running against the OES NCP server so we had to switch to CIFS in a hurry and whilst it does not fall over in a heap like ncpfs does and we now have working reconnects when the home directory server is rebooted (home directories magically start working again after the reboot which is brilliant!) many applications do not work.
>
> I can only assume this is because the CIFS server is an abomination rather than that the CIFS modules is quite that badly broken…  Anyway, my question is: do you want patches that make the CIFS module work better with OES CIFS server or do you not care?
>
> For example lots of applications require working hard links so I had to port our Virtual Hard Link implementation from NCPfs to CIFS to get hard links working which fixed a lot of applications but then we started hitting real bugs.  A few examples:
>
> - llseek(SEEK_END) fails against the OES server with error -EBADF from the server because the SEEK_END causes file revalidation which ends up calling CIFSSMBQFileInfo() which then results in the -EBADF.  I wrote a patch to fall back to path based query via CIFSSMBQPathInfo() and if that fails via SMBQueryInformation() and now the llseek(SEEK_END) works thus applications like Seamonkey actually manage to launch rather than segfaulting.
>
> - mkdir() returns -EACCES when the target exists.  Again this is coming from the OES server.  I wrote a patch that calls cifs_get_inode_info() when CIFSSMBMkDir() returns -EACCES and unix extensions are not enabled on the superblock and if the result is success I rewrite the return code to -EEXIST which fixes the mkdir issue and that fixes loads of Gnome related applications.
>
> - The OES server behaves like NT4 in that it returns success from CIFSSMBSetPathInfo() but it actually ignores the call unless the file is open on the server.  But the OES server does not have CIFS_SES_NT4 in the session flags thus the work around in the CIFS module does not trigger and thus changing the access times or permissions does not work unless the file happens to be open which breaks applications like rsync.  I commented the CIFSSMBSetPathInfo() call completely in a patch so it falls back to the other code which makes it work for us.
>
> And finally the actual question: Are you interested in any of these patches or at least are you interested in fixing these issues in the standard CIFS module in which case I can send you my patches or at least give you detailed descriptions of what is failing and how?

Yes - we are strongly interested in making sure that the cifs module
is stable (always) against all servers (this is much harder than it
sounds) and works well against the major servers.   We travel to the
two major test events each year testing against the major servers (and
NAS).   For less functional servers, we are always interested in
stability fixes and as long as they are not too invasive, will
consider some workaround fixes to deal with server bugs if the patches
aren't high risk.

-- 
Thanks,

Steve

Re: CIFS kernel module bug

[Jeremy Allison]

On Fri, Sep 30, 2011 at 10:30:47PM +0100, Anton Altaparmakov wrote:
> 
> Yes, I am happy to do that.
> 
> Also a question for you: I am up to the neck in having to adapt the CIFS module as it doesn't work very well against the Novell Open Enterprise Server CIFS server (that is why I was looking at the CIFS code and noticed this logic bug in the first place).
> 
> A bit of background: the home directories for our (i.e. University of Cambridge Computing Service) Linux distribution "MCS Linux" which runs on about a thousand workstations and a handfull of remote access servers used to be on Netware and were moved to OES Linux recently and the ncpfs kernel module really fell over in a heap when running against the OES NCP server so we had to switch to CIFS in a hurry and whilst it does not fall over in a heap like ncpfs does and we now have working reconnects when the home directory server is rebooted (home directories magically start working again after the reboot which is brilliant!) many applications do not work.
> 
> I can only assume this is because the CIFS server is an abomination rather than that the CIFS modules is quite that badly broken…  Anyway, my question is: do you want patches that make the CIFS module work better with OES CIFS server or do you not care?
> 
> For example lots of applications require working hard links so I had to port our Virtual Hard Link implementation from NCPfs to CIFS to get hard links working which fixed a lot of applications but then we started hitting real bugs.  A few examples:
> 
> - llseek(SEEK_END) fails against the OES server with error -EBADF from the server because the SEEK_END causes file revalidation which ends up calling CIFSSMBQFileInfo() which then results in the -EBADF.  I wrote a patch to fall back to path based query via CIFSSMBQPathInfo() and if that fails via SMBQueryInformation() and now the llseek(SEEK_END) works thus applications like Seamonkey actually manage to launch rather than segfaulting.
> 
> - mkdir() returns -EACCES when the target exists.  Again this is coming from the OES server.  I wrote a patch that calls cifs_get_inode_info() when CIFSSMBMkDir() returns -EACCES and unix extensions are not enabled on the superblock and if the result is success I rewrite the return code to -EEXIST which fixes the mkdir issue and that fixes loads of Gnome related applications.
> 
> - The OES server behaves like NT4 in that it returns success from CIFSSMBSetPathInfo() but it actually ignores the call unless the file is open on the server.  But the OES server does not have CIFS_SES_NT4 in the session flags thus the work around in the CIFS module does not trigger and thus changing the access times or permissions does not work unless the file happens to be open which breaks applications like rsync.  I commented the CIFSSMBSetPathInfo() call completely in a patch so it falls back to the other code which makes it work for us.
> 
> And finally the actual question: Are you interested in any of these patches or at least are you interested in fixing these issues in the standard CIFS module in which case I can send you my patches or at least give you detailed descriptions of what is failing and how?

What server code is the OES CIFS server running ? I thought Novell CIFS
services were all Samba based.

Jeremy.

Re: CIFS kernel module bug

[J.P. King]

Anton will be asleep, so I'll attempt to answer this one.

> What server code is the OES CIFS server running ? I thought Novell CIFS
> services were all Samba based.

We have been told by the people upstairs who run the server that it isn't. 
I have a recollection that it may be a forked version of the samba code, 
but don't rely on my recollections.

Either way it is sitting on top of NSS, a Novell filesystem, which is 
significantly different to "normal" Linux filesystem.  I don't know that
this is the issue, but it is a difference which may affect things 
regardless of the fileserver code.


> Jeremy.

Julian
--
Julian King
Computer Officer, University of Cambridge, Unix Support

Re: CIFS kernel module bug

[Anton Altaparmakov]

Hi,

On 1 Oct 2011, at 01:34, J.P. King wrote:
> Anton will be asleep, so I'll attempt to answer this one.
> 
>> What server code is the OES CIFS server running ? I thought Novell CIFS
>> services were all Samba based.
> 
> We have been told by the people upstairs who run the server that it isn't.

It is not Samba based.  Like Apple, Novell have written their own since Samba decided to go GPLv3…

> I have a recollection that it may be a forked version of the samba code, but don't rely on my recollections.

It is not.  Or it better not be given the license is binary only!!!  It would have to be GPL if it were forked.

> Either way it is sitting on top of NSS, a Novell filesystem, which is significantly different to "normal" Linux filesystem.  I don't know that
> this is the issue, but it is a difference which may affect things regardless of the fileserver code.


I had a quick google and Novell's own OES documentation for CIFS has a section titled:

	"Comparing Novell CIFS and Novell Samba"

If you care, it is on page 77 of this PDF:

	http://www.novell.com/documentation/oes2/pdfdoc/file_cifs_lx/file_cifs_lx.pdf

And our filestore is running the Novell CIFS server, not the Novell Samba one in case that wasn't obvious from previous discussion…

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/

Re: CIFS kernel module bug

[Lars Müller]

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

On Sat, Oct 01, 2011 at 09:48:12AM +0100, Anton Altaparmakov wrote:
> 
> On 1 Oct 2011, at 01:34, J.P. King wrote:
> > Anton will be asleep, so I'll attempt to answer this one.
> > 
> >> What server code is the OES CIFS server running ? I thought Novell CIFS
> >> services were all Samba based.
> > 
> > We have been told by the people upstairs who run the server that it isn't.
> 
> It is not Samba based.  Like Apple, Novell have written their own since Samba decided to go GPLv3…

(Potential) FUD alert. ;)

This Novell CIFS server existed before Novell bought SUSE.  And the
decission to go with it instead of Samba had nothing to do with the move
of Samba from GPL v2 to v3.

Lars
-- 
Lars Müller [ˈlaː(r)z ˈmʏlɐ]
Samba Team
SUSE Linux, Maxfeldstraße 5, 90409 Nürnberg, Germany

[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]

Re: CIFS kernel module bug

[Anton Altaparmakov]

Hi Lars,

On 1 Oct 2011, at 16:07, Lars Müller wrote:
> On Sat, Oct 01, 2011 at 09:48:12AM +0100, Anton Altaparmakov wrote:
>> 
>> On 1 Oct 2011, at 01:34, J.P. King wrote:
>>> Anton will be asleep, so I'll attempt to answer this one.
>>> 
>>>> What server code is the OES CIFS server running ? I thought Novell CIFS
>>>> services were all Samba based.
>>> 
>>> We have been told by the people upstairs who run the server that it isn't.
>> 
>> It is not Samba based.  Like Apple, Novell have written their own since Samba decided to go GPLv3…
> 
> (Potential) FUD alert. ;)

It wasn't deliberate!  It was just a guess as until recently I also though that OES just used Samba so when I two weeks ago found out that it didn't I just assumed that Novell must have written it recently and then drew the parallel to Apple and just assumed that it was an analogous case…

> This Novell CIFS server existed before Novell bought SUSE.  And the
> decission to go with it instead of Samba had nothing to do with the move
> of Samba from GPL v2 to v3.

I stand corrected.  Thanks for the clarification!  From your signature I guess you would know a lot better than most.  (-:

Best regards,

	Anton

> Lars
> -- 
> Lars Müller [ˈlaː(r)z ˈmʏlɐ]
> Samba Team
> SUSE Linux, Maxfeldstraße 5, 90409 Nürnberg, Germany

-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/

Re: CIFS kernel module bug

[Jim McDonough]

On Sat, Oct 1, 2011 at 3:53 PM, Anton Altaparmakov <aia21@cam.ac.uk> wrote:
> Hi Lars,
>
> On 1 Oct 2011, at 16:07, Lars Müller wrote:
>> On Sat, Oct 01, 2011 at 09:48:12AM +0100, Anton Altaparmakov wrote:
>>>
>>> On 1 Oct 2011, at 01:34, J.P. King wrote:
>>>> Anton will be asleep, so I'll attempt to answer this one.
>>>>
>>>>> What server code is the OES CIFS server running ? I thought Novell CIFS
>>>>> services were all Samba based.
>>>>
>>>> We have been told by the people upstairs who run the server that it isn't.
>>>
>>> It is not Samba based.  Like Apple, Novell have written their own since Samba decided to go GPLv3…
>>
>> (Potential) FUD alert. ;)
>
> It wasn't deliberate!  It was just a guess as until recently I also though that OES just used Samba so when I two weeks ago found out that it didn't I just assumed that Novell must have written it recently and then drew the parallel to Apple and just assumed that it was an analogous case…

I believe both are available on OES, samba and the CIFS server that
has its origins in Netware.

>
>> This Novell CIFS server existed before Novell bought SUSE.  And the
>> decission to go with it instead of Samba had nothing to do with the move
>> of Samba from GPL v2 to v3.
>
> I stand corrected.  Thanks for the clarification!  From your signature I guess you would know a lot better than most.  (-:
>
> Best regards,
>
>        Anton
>
>> Lars
>> --
>> Lars Müller [ˈlaː(r)z ˈmʏlɐ]
>> Samba Team
>> SUSE Linux, Maxfeldstraße 5, 90409 Nürnberg, Germany
>
> --
> Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
> Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
> Linux NTFS maintainer, http://www.linux-ntfs.org/
>
>



-- 
Jim McDonough
Samba Team
SUSE labs
jmcd at samba dot org
jmcd at themcdonoughs dot org

Re: CIFS kernel module bug

[Steve French]

On Sat, Oct 1, 2011 at 3:22 PM, Jim McDonough <jmcd@samba.org> wrote:
> On Sat, Oct 1, 2011 at 3:53 PM, Anton Altaparmakov <aia21@cam.ac.uk> wrote:
>> Hi Lars,
>>
>> On 1 Oct 2011, at 16:07, Lars Müller wrote:
>>> On Sat, Oct 01, 2011 at 09:48:12AM +0100, Anton Altaparmakov wrote:
>>>>
>>>> On 1 Oct 2011, at 01:34, J.P. King wrote:
>>>>> Anton will be asleep, so I'll attempt to answer this one.
>>>>>
>>>>>> What server code is the OES CIFS server running ? I thought Novell CIFS
>>>>>> services were all Samba based.
>>>>>
>>>>> We have been told by the people upstairs who run the server that it isn't.
>>>>
>>>> It is not Samba based.  Like Apple, Novell have written their own since Samba decided to go GPLv3…
>>>
>>> (Potential) FUD alert. ;)
>>
>> It wasn't deliberate!  It was just a guess as until recently I also though that OES just used Samba so when I two weeks ago found out that it didn't I just assumed that Novell must have written it recently and then drew the parallel to Apple and just assumed that it was an analogous case…
>
> I believe both are available on OES, samba and the CIFS server that
> has its origins in Netware.
>
>>
>>> This Novell CIFS server existed before Novell bought SUSE.  And the
>>> decission to go with it instead of Samba had nothing to do with the move
>>> of Samba from GPL v2 to v3.
>>
>> I stand corrected.  Thanks for the clarification!  From your signature I guess you would know a lot better than most.  (-:
>>
>> Best regards,
>>
>>        Anton
>>
>>> Lars
>>> --
>>> Lars Müller [ˈlaː(r)z ˈmʏlɐ]
>>> Samba Team
>>> SUSE Linux, Maxfeldstraße 5, 90409 Nürnberg, Germany
>>
>> --
>> Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
>> Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
>> Linux NTFS maintainer, http://www.linux-ntfs.org/

Key point is - we will fix the Linux cifs kernel client to make
reasonable workarounds for server bugs for something like this -
especially if the server is easy to download and/or test against (and
assuming it supports reasonable security - ntlmv2 and/or kerberos)


-- 
Thanks,

Steve

Re: CIFS kernel module bug

[Anton Altaparmakov]

Hi,

On 1 Oct 2011, at 21:22, Jim McDonough wrote:
> On Sat, Oct 1, 2011 at 3:53 PM, Anton Altaparmakov <aia21@cam.ac.uk> wrote:
>> On 1 Oct 2011, at 16:07, Lars Müller wrote:
>>> On Sat, Oct 01, 2011 at 09:48:12AM +0100, Anton Altaparmakov wrote:
>>>> 
>>>> On 1 Oct 2011, at 01:34, J.P. King wrote:
>>>>> Anton will be asleep, so I'll attempt to answer this one.
>>>>> 
>>>>>> What server code is the OES CIFS server running ? I thought Novell CIFS
>>>>>> services were all Samba based.
>>>>> 
>>>>> We have been told by the people upstairs who run the server that it isn't.
>>>> 
>>>> It is not Samba based.  Like Apple, Novell have written their own since Samba decided to go GPLv3…
>>> 
>>> (Potential) FUD alert. ;)
>> 
>> It wasn't deliberate!  It was just a guess as until recently I also though that OES just used Samba so when I two weeks ago found out that it didn't I just assumed that Novell must have written it recently and then drew the parallel to Apple and just assumed that it was an analogous case…
> 
> I believe both are available on OES, samba and the CIFS server that
> has its origins in Netware.

That is correct but apparently Salford Software (who handle our Netware & OES site license here in the UK including providing support) have told the guys running the file server that the samba server on OES is not scalable, has loads of bugs, and they must stay away from it at all costs and that they would not provide any support if our filestore runs that instead of the Novell CIFSd.

You can understand why the filestore is thus running the Novell CIFSd rather than Samba…

PS. Whether this is FUD or not I cannot comment on.  I am just passing on what I was told!

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/

Re: CIFS kernel module bug

[Jeremy Allison]

On Sun, Oct 02, 2011 at 09:16:33PM +0100, Anton Altaparmakov wrote:
> 
> That is correct but apparently Salford Software (who handle our Netware & OES site license here in the UK including providing support) have told the guys running the file server that the samba server on OES is not scalable, has loads of bugs, and they must stay away from it at all costs and that they would not provide any support if our filestore runs that instead of the Novell CIFSd.
> 
> You can understand why the filestore is thus running the Novell CIFSd rather than Samba…
> 
> PS. Whether this is FUD or not I cannot comment on.  I am just passing on what I was told!

Wow. And Samba being (partly) a UK-written project too (at least
 from me :-).

I think the Samba Team members from Novell who work full time
on said Samba server in OES might have some follow-ups here,
so I think I'll let them do that.

FYI. Just for fun, try running smbtorture4 against the CIFSd
thing you're running now. I'll be (probably) grimly amused by
the results. A warning - don't do that on a production system :-).

Jeremy.

Re: CIFS kernel module bug

[Lars Müller]

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

Hi,

On Sun, Oct 02, 2011 at 09:16:33PM +0100, Anton Altaparmakov wrote:
> 
> On 1 Oct 2011, at 21:22, Jim McDonough wrote:
[ 8< ]
> > I believe both are available on OES, samba and the CIFS server that
> > has its origins in Netware.
> 
> That is correct but apparently Salford Software (who handle our Netware & OES site license here in the UK including providing support) have told the guys running the file server that the samba server on OES is not scalable, has loads of bugs, and they must stay away from it at all costs and that they would not provide any support if our filestore runs that instead of the Novell CIFSd.
> 
> You can understand why the filestore is thus running the Novell CIFSd rather than Samba…

This OES is very likely SUSE Linux Enterprise (SLE) 10 based.  This code
base initially included Samba 3.0.

There is an additional, optional software repository available online
for SUSE Linux Enterprise customers - named SLES10-GPLv3-Extras - which
provides Samba 3.4.3[0] - same version and including the same fixes as
the version used with SLE 11 SP 1.

Ok, that's the good news.  Now to the potential bad part.

The bad part only applies if you make use of Domain Services for Windows
(DSfW) formerly known as xad.

Some of the required modifications had not been ported to Samba post-3.0
and therefore running any post-3.0 Samba code will cause trouble aka
will not work.  We're working on this with the OES colleagues.

Lars

[0]
http://www.novell.com/support/viewContent.do?externalId=7007836&sliceId=1
-- 
Lars Müller [ˈlaː(r)z ˈmʏlɐ]
Samba Team
SUSE Linux, Maxfeldstraße 5, 90409 Nürnberg, Germany

[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]

Re: CIFS kernel module bug

[Jeff Layton]

On Fri, 30 Sep 2011 22:30:47 +0100
Anton Altaparmakov <aia21@cam.ac.uk> wrote:

> Hi,
> 
> On 30 Sep 2011, at 19:04, Jeff Layton wrote:
> > On Fri, 30 Sep 2011 14:58:58 +0100 Anton Altaparmakov <aia21@cam.ac.uk> wrote:
> > 
> >> Hi,
> >> 
> >> Looking at the current kernel (in Linus' repository on github) there is a silly logic bug in the cifs module in fs/cifs/cifsfs.c::cifs_llseek() there is this bit of code:
> >> 
> >> 	/*
> >> 	 * origin == SEEK_END || SEEK_DATA || SEEK_HOLE => we must revalidate
> >> 	 * the cached file length
> >> 	 */
> >> 	if (origin != SEEK_SET || origin != SEEK_CUR) {
> >> 
> >> The logical or should be a logical and, i.e. this should be:
> >> 
> >> 	if (origin != SEEK_SET && origin != SEEK_CUR) {
> >> 
> >> As the code is at present that line is ALWAYS true because origin is ALWAYS either != SEEK_SET or != SEEK_CUR as if it equals one it cannot equal the other and vice versa…
> >> 
> >> So at the moment it always does the revalidation instead of only for SEEK_END, SEEK_DATA, and SEEK_HOLE.
> > 
> > Haha, good catch. Care to roll up a patch to fix that?
> 
> 
> Yes, I am happy to do that.
> 
> Also a question for you: I am up to the neck in having to adapt the CIFS module as it doesn't work very well against the Novell Open Enterprise Server CIFS server (that is why I was looking at the CIFS code and noticed this logic bug in the first place).
> 
> A bit of background: the home directories for our (i.e. University of Cambridge Computing Service) Linux distribution "MCS Linux" which runs on about a thousand workstations and a handfull of remote access servers used to be on Netware and were moved to OES Linux recently and the ncpfs kernel module really fell over in a heap when running against the OES NCP server so we had to switch to CIFS in a hurry and whilst it does not fall over in a heap like ncpfs does and we now have working reconnects when the home directory server is rebooted (home directories magically start working again after the reboot which is brilliant!) many applications do not work.
> 
> I can only assume this is because the CIFS server is an abomination rather than that the CIFS modules is quite that badly broken…  Anyway, my question is: do you want patches that make the CIFS module work better with OES CIFS server or do you not care?
> 
> For example lots of applications require working hard links so I had to port our Virtual Hard Link implementation from NCPfs to CIFS to get hard links working which fixed a lot of applications but then we started hitting real bugs.  A few examples:
> 
> - llseek(SEEK_END) fails against the OES server with error -EBADF from the server because the SEEK_END causes file revalidation which ends up calling CIFSSMBQFileInfo() which then results in the -EBADF.  I wrote a patch to fall back to path based query via CIFSSMBQPathInfo() and if that fails via SMBQueryInformation() and now the llseek(SEEK_END) works thus applications like Seamonkey actually manage to launch rather than segfaulting.
> 

Yeah, that sounds like a server bug, but maybe it's not expected to
support QUERY_FILE_INFO calls for some reason? Some network traces
might help clarify the issue here.

> - mkdir() returns -EACCES when the target exists.  Again this is coming from the OES server.  I wrote a patch that calls cifs_get_inode_info() when CIFSSMBMkDir() returns -EACCES and unix extensions are not enabled on the superblock and if the result is success I rewrite the return code to -EEXIST which fixes the mkdir issue and that fixes loads of Gnome related applications.
> 

Probably this is a bad mapping of this error in cifs.ko. Again, a
network trace might be helpful.

> - The OES server behaves like NT4 in that it returns success from CIFSSMBSetPathInfo() but it actually ignores the call unless the file is open on the server.  But the OES server does not have CIFS_SES_NT4 in the session flags thus the work around in the CIFS module does not trigger and thus changing the access times or permissions does not work unless the file happens to be open which breaks applications like rsync.  I commented the CIFSSMBSetPathInfo() call completely in a patch so it falls back to the other code which makes it work for us.
> 

We probably can't take this patch as-is, but it's possible you could
try to enable this workaround via some other mechanism too.

> And finally the actual question: Are you interested in any of these patches or at least are you interested in fixing these issues in the standard CIFS module in which case I can send you my patches or at least give you detailed descriptions of what is failing and how?
> 


Yes, it wouldn't hurt to send them to the list so we can debate them.
We certainly are not opposed to adding reasonable workarounds for
problem servers.

-- 
Jeff Layton <jlayton@samba.org>