Bug 22089 - [PATCH] locale: Call fchmod() before calling link()
Summary: [PATCH] locale: Call fchmod() before calling link()
Status: UNCONFIRMED
Alias: None
Product: glibc
Classification: Unclassified
Component: locale (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-05 21:33 UTC by Colin
Modified: 2018-01-03 13:46 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
[PATCH] locale: Call fchmod() before calling link() (1.01 KB, patch)
2017-09-05 21:33 UTC, Colin
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Colin 2017-09-05 21:33:46 UTC
Created attachment 10395 [details]
[PATCH] locale: Call fchmod() before calling link()

From d25853801c2a3cb8357b75b750af12bc6b43f75f Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Tue, 5 Sep 2017 16:55:34 -0400
Subject: [PATCH] locale: Call fchmod() before calling link()

The [rpm-ostree](https://github.com/projectatomic/rpm-ostree)
project implements atomic, online system upgrades using
[libostree](https://github.com/ostreedev/ostree/), which
contains a `rofiles-fuse` program:
https://github.com/ostreedev/ostree/blob/master/man/rofiles-fuse.xml

Currently rpm-ostree will run all scripts, such as glibc's
`build-locale-archive` with an rofiles-fuse mount active. This fails
because we call `fchmod()` *after* calling `link()`, and `rofiles-fuse`
rejects writes to files with multiple links.

There's no reason not to call `fchmod()` beforehand in all cases, so let's do so
and fix the `rofiles-fuse` case.
---
 ChangeLog                    |  5 +++++
 locale/programs/locarchive.c | 18 +++++++++---------
 2 files changed, 14 insertions(+), 9 deletions(-)
Comment 1 Colin 2017-12-08 20:45:39 UTC
Any comments on this patch?  It's still relevant for rpm-ostree, although we *might* cave and implement copyup in rofiles-fuse, I think this patch is still clearly right.
Comment 2 Dmitry V. Levin 2017-12-08 21:06:41 UTC
(In reply to Colin from comment #1)
> Any comments on this patch?  It's still relevant for rpm-ostree, although we
> *might* cave and implement copyup in rofiles-fuse, I think this patch is
> still clearly right.

If you are proposing a patch, please submit it to libc-alpha mailing list (see https://sourceware.org/glibc/wiki/Contribution%20checklist for more information).
Comment 3 Dmitry V. Levin 2017-12-08 21:10:14 UTC
(In reply to Colin from comment #0)
> Currently rpm-ostree will run all scripts, such as glibc's
> `build-locale-archive` with an rofiles-fuse mount active. This fails
> because we call `fchmod()` *after* calling `link()`, and `rofiles-fuse`
> rejects writes to files with multiple links.

What the current code does is essentially the following:
 fd = mkstemp (fname);
 link (fname, archivefname);
 unlink (fname);
 fchmod (fd, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);

How could this lead to creation of a file with multiple links?
Comment 4 Colin 2018-01-03 13:46:37 UTC
Hm, it might be that we see a different order of operations via FUSE?  Anyways even if we landed this patch we'd still need to support older glibc versions, so I'm going to cave and implement copyup: https://github.com/ostreedev/ostree/pull/1382

That said I still think this patch is a cleanup, although an even better cleanup would be to use O_TMPFILE.