linux-kernel - Re: [PATCH v2 1/2] um: Set secure access mode for temporary file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADNZ+wTtu5gZuztSAnY9TE0fV4cVgqr534gyDUEkp0LS=Vq4Xw@mail.gmail.com>
Date:	Tue, 8 Dec 2015 15:37:39 -0500
From:	Tristan Schmelcher <tschmelcher@...gle.com>
To:	Mickaël Salaün <mic@...ikod.net>
Cc:	linux-kernel@...r.kernel.org, Jeff Dike <jdike@...toit.com>,
	Richard Weinberger <richard@....at>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	user-mode-linux-devel <user-mode-linux-devel@...ts.sourceforge.net>,
	user-mode-linux-user@...ts.sourceforge.net
Subject: Re: [PATCH v2 1/2] um: Set secure access mode for temporary file

On 6 December 2015 at 09:43, Mickaël Salaün <mic@...ikod.net> wrote:
> Well, I'm concerned to use umask because it is not thread-safe and drivers may use create_mem_file() in a multi-theaded context.

You are right. We should perhaps set the umask to 0700 permanently
during process start. But I am not sure if this will interfere with
other UML code.

> I prefer to stick to fchmod and handle the race-condition with O_TMPFILE unsell someone is sure that this will not create bugs :)

The fchmod call is basically useless and should probably be removed.
Even mmap only checks the file descriptor, not the file permissions. I
have pasted a test program below if you wish to confirm. AFAICT
changing the permissions after file deletion accomplishes nothing
unless the attacker bizarrely chooses to hard-link the file during the
race instead of opening it.

#include <assert.h>
#include <fcntl.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

int main(int argc, char **argv) {
  int fd = open("./foo", O_RDWR|O_CREAT|O_EXCL, 0700);
  assert(fd >= 0);
  int ret = write(fd, "bar\n", 4);
  assert(ret == 4);
  ret = fchmod(fd, 0400);
  assert(ret >= 0);
  char *buf = mmap(0, 4, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_SHARED, fd, 0);
  assert(buf);
  buf[2] = 'z';
  ret = munmap(buf, 4);
  assert(ret >= 0);
  return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/