[systemd-devel] [systemd-commits] 9 commits - configure.ac .gitignore Makefile.am Makefile-man.am man/systemd-fsckd.service.xml man/systemd-fsck at .service.xml po/de.po po/el.po po/fr.po po/hu.po po/it.po po/pl.po po/POTFILES.in po/pt_BR.po po/ru.po po/sv.po po/uk.po src/fsck src/fsckd src/shared test/mocks units/.gitignore units/systemd-fsckd.service.in units/systemd-fsckd.socket units/systemd-fsck-root.service.in units/systemd-fsck at .service.in

[Lennart Poettering]

On Tue, 10.03.15 08:33, Martin Pitt (martin.pitt at ubuntu.com) wrote:

> Lennart Poettering [2015-03-09 19:11 +0100]:
> > Anyway, please look into fixing this, I am kinda relying on patches
> > for this, as I don't use this myself. Fedora isn't set up for it, nor
> > do I use a file system that still requires fsck at boot...
> 
> Yep, we'll fix those. But for the record, this can be tested easily
> anywhere by replacing /usr/sbin/fsck with test/mocks/fsck (I'm on
> btrfs myself..)

Hmm, is there also a ply mocker? That would be good!

> Lennart Poettering [2015-03-09 19:45 +0100]:
> > And in a similar way client_progress_handler() is hosed too. Even
> > worse: if a client sends messages byte-wise (which is absolutely OK on
> > SOCK_STREAM) it will be kicked off the connection.
> 
> We did talk about using a SOCK_DGRAM socket, but for some reason they
> can only be used in a connectionless mode on the server, i. e. you
> cannot listen() on them. This means that s-fsckd will never know in a
> timely fashion when a client disconnected or died, so you can only
> stop doing stuff after some generously long timeout. That's why this
> uses a SOCK_STREAM, but it does use send() and recv() to send messages
> in one block. To guard against broken/malicious clients, fsckd kicks
> connections which send malformed data.
> 
> This could potentially made more explicit by using SOCK_SEQPACKET;
> we'd still need to check for short/malformed data of course, so that
> check can't go away entirely. But it at least stops clients trying to
> open in stream mode and send characters. But the only client here is
> our own s-fsck, so this is all internal anyway..

SOCK_SEQPACKET is what you want, it's connection based, as knows
datagrams.

That said, I don't think this should be necessary at all: fsckd should
be connected directly with each fsck's stdout and parse the raw data
directly, which means SOCK_STREAM needs to be used as stdout should be
a stream fd.

> 
> That didn't actually come up during review, but that sounds like a
> nice idea! I see two structures:
> 
>  - s-fsck runs fsck and forwards its stdout/err fds to fsckd, and then
>    fsckd does all the parsing. This would also automatically eliminate
>    the intermediate socket handling from above.

Well, again: fsck should *not* 'forward'. It should not be bothered at
all with processing stdout of the actual fsck tool at all. Instead it
should open a AF_UNIX/SOCK_STREAM connection to fsckd, use shutdown()
to close the read side, and then make the resulting fd the fd passed
to fsck's -C parameter. That way, the fsck tool will pass the progress
data *directly* to systemd-fsckd, and systemd-fsck will never see it
at all!

> Or, more radically:
> 
>  - eliminate the current functionality of s-fsck and just make that
>    send a request to s-fsckd to check a new device name.  s-fsckd
>    would then spawn off /usr/sbin/fsck by itself and start parsing its
>    output. That's more complex handling of its children (but not too
>    bad), and completely avoids passing around data through sockets,
>    and s-fsck would be a trivial five-liner.
> 
> Does that sound better/easier?

I think it would be more obvious for admins if fsck stays part of the
systemd-fsck at .service services, so that its resources/logs and so on
are accounted to it, rather than a centralized fsckd.service.

Lennart

-- 
Lennart Poettering, Red Hat