[PATCH] gensignkey: ensure host keys are flushed to disk

Peter Korsgaard jacmet at sunsite.dk
Mon Dec 1 19:51:08 AWST 2014


>>>>> "DELOGET," == DELOGET, Emmanuel <emmanuel.deloget at sfr.com> writes:

Hi, (this time from a subscribed address)

>> Thanks for the patch, I've applied it with small changes.
 >> https://secure.ucc.asn.au/hg/dropbear/rev/fd2e8bbb0333
 >> 
 >> Emmanuel - thanks for the review. Dropbear already has
 >> exit-on-failure m_strdup(), I'm using that. I'll avoid
 >> O_DIRECTORY since it's fairly harmless to leave out and is a
 >> portability hassle. I've made it open with O_RDWR though can't
 >> actually see in the opengroup.org posix docs where that is
 >> required for fsync?

 > Part of the problem has its root here: the POSIX standard does not say a
 > word on this specific issue - so different UNIX have different
 > implementation. I found at least two of them:

 > http://nixdoc.net/man-pages/irix/man2/fsync.2.html
 > http://nixdoc.net/man-pages/Tru64/man2/fsync.2.html

 > Granted, there are not the primary target for dropbear :)

 > The linux man page warns about that issue in the Notes section:

 > http://linux.die.net/man/2/fsync

 > Additionnally, a good number of other UNIX (and other POSIX layers such
 > as the one offered by eCos) doesn't say anything about that so it's hard
 > to know how the implementation behaves without testing it.

 > I might be a bit over-prudent here and maybe noone will ever notice. But
 > since it can lead to a nasty bug I think it's better to err on the side
 > of safety :)

Actually it is the other way around. Something related came up today,
and I finally now got to test what got committed. It doesn't work (on
Linux atleast) as open on the parent directory fails with -EISDIR unless
it is opened read only. This is also documented in the man page:

EISDIR pathname refers to a directory and the access requested involved
writing (that is, O_WRONLY or O_RDWR is set).

The exact code handling it in the kernel is in fs/namei.c:

static int may_open(struct path *path, int acc_mode, int flag)
{
        struct dentry *dentry = path->dentry;
        struct inode *inode = dentry->d_inode;
        int error;

        /* O_PATH? */
        if (!acc_mode)
                return 0;

        if (!inode)
                return -ENOENT;

        switch (inode->i_mode & S_IFMT) {
        case S_IFLNK:
                return -ELOOP;
        case S_IFDIR:
                if (acc_mode & MAY_WRITE)
                        return -EISDIR;
                break;
        ...

So we either need to change it back to O_RDONLY, or first try with
O_RDWR and fall back to O_RDONLY if it fails with EISDIR in case other
OSes require RDWR access for fsync.

What are we going to do?

-- 
Bye, Peter Korsgaard


More information about the Dropbear mailing list