Message-ID: <CAH2r5mtgV=NkZVChDY-apdqkvM9KFkraRGy_jDCpLmFU6PFMAA@mail.gmail.com>
Date: Sat, 14 Sep 2024 01:21:17 -0500
From: Steve French <smfrench@...il.com>
To: Pali Rohár <pali@...nel.org>
Cc: Steve French <sfrench@...ba.org>, Paulo Alcantara <pc@...guebit.com>, 
	Ronnie Sahlberg <ronniesahlberg@...il.com>, linux-cifs@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files

On Fri, Sep 13, 2024 at 5:42 PM Pali Rohár <pali@...nel.org> wrote:
>
> On Friday 13 September 2024 17:14:22 Steve French wrote:
> > How did you find the format of the FIFO and SOCK file types?  I
>
> For fifo there are multiple sources on internet, but none of them is
> normative. Everything is just what people have tried. For example this
> old email on samba list:
> https://lists.samba.org/archive/linux-cifs-client/2005-May/000856.html
>
> Format of the socket I have figured out by creating it in Interix
> subsystem and then dumped content of the file from Win32 subsystem.
> Then I checked that it has also same format over older MS NFS server.
> It was easier than trying to search for some documentation (which I have
> not found).
>
> > couldn't find any references to those so added two new types to allow
> > current Linux to be able to create these (especially since they are
> > opaque to the server and thus low risk).
>
> I was searching over internet again and now I have found patent
> https://patents.google.com/patent/US20090049459 which describe symlink
> content:
>
> #define NFS_SPECFILE_LNK_V1  0x014b4e4c78746e49 /* “IntxLNK” */
>
> But does not describe other types.
>
> > > +     case S_IFSOCK:
> > > -             strscpy(pdev.type, "LnxSOCK");
> > > +             /* SFU socket is system file with one zero byte */
> > > +             pdev_len = 1;
> > > +             pdev.type[0] = '\0';
> > >               break;
> > >       case S_IFIFO:
> > > -             strscpy(pdev.type, "LnxFIFO");
> > > +             /* SFU fifo is system file which is empty */
> > > +             pdev_len = 0;
> >
> > My worry about the suggested change above is that it is possible that
> > we could accidentally match to an empty file.
>
> I fully understand your concerns, but code in this patch is for creating
> new fifos. Not recognizing existing fifos.
>
> Code for recognizing existing fifos (=empty file with system attribute)
> was not changed and is in Linux cifs client for a very long time.
<>
> > We intentionally added
> > the two new dev.type fields for these to avoid collisions with other
> > things (and since they are Linux specific).  It seems risky to have an
> > empty file with the system attribute marked as a FIFO, and similarly a
> > file with one byte null as Socket.   Since this is for only the Linux
> > client to recognize, I wanted to do something safe for those file
> > types less likely to be confusing (ie strings for Socket and FIFO that
> > were similar in length and format to the other special files seemed
> > intuitive) and "LnxFIFO" and LnxSOCK" were used as the tags in the
> > file to reduce confusion ie the tags for those two start with "Lnx" -
> > ie "something used for Linux client" not related to the original
> > Interix (those begin with "Intx").
>
> I see. Now I understand what are those types (as I have not seen them
> before). It is somehow misleading if such "LnxFIFO" and LnxSOCK"
> functionality is provided by SFU option, but is incompatible with MS SFU
> and also with MS NFS server. And is also incompatible with older Linux
> cifs clients (as they do not understand those Lnx types).

I am not as worried about FIFO and SOCK type being recognized by
older servers (since almost every use case for them would be for them
to be seen (only) by the client - e.g. for mounts to servers that
don't implement reparse points yet), and since they are less
common file types I am willing to let them be unrecognized by
old clients (we can tag them for stable if older distros don't have
them),  but I am concerned about allowing "current clients" to
create empty files for an unusual purpose which could be
confusing/non-intuitive.

And since this change (at least the one to allow FIFOs to be created with "sfu"
has been in mainline for a year and also since it uses a more intuitive tag
("LnxFIFO") than the empty one used by very old Windows) the only
clients who would have created these would be already using this newer tag
(older Linux clients couldn't have created such files - there seems more
risk of regression with reverting the change than with continuing with
the Linux client specific tag (at least to the one for FIFOs
since that has been in much longer than the socket one which is recent)

Will discuss with others - opinions welcome.

There is an upcoming SMB3.1.1 test event coming up next week (and the annual
Storage Developer Conference too) so I can see if others have opinions one
way or another on whether to move to empty (or 1 byte) files for
creating fifos/sockets

> > Note that in the long run we hope to use reparse points by default in
> > more servers to store special files like this but there are a few
> > cases for unusual workloads that need special file support that would
> > have to use sfu still.  The newer reparse tags that Windows uses "WSL"
> > have the advantage that they require fewer roundtrips to query (since
> > the file type is in the reparse tag).
>
> Yes, new WSL tags seems to be better. Also SFU mount option is not
> activated by default.
>
> > Also noticed an interesting problem when mounted with "sfu" -
> > "smbgetinfo filebasicinfo /mnt/fifo1" will hang (in sys_open).  Is
> > that expected for a FIFO?
>
> Reading from fifo sleep reading process until some other process write
> data to fifo. This is how fifos are working. You can try it on local
> filesystem (e.g. ext4 or tmpfs).

makes sense - thx

Powered by blists - more mailing lists