RFR : 8211326 : add OS user related information to hs_err file
Baesken, Matthias
matthias.baesken at sap.com
Fri Nov 9 10:08:27 UTC 2018
Hi Thomas ,
http://man7.org/linux/man-pages/man2/umask.2.html
says :
It is impossible to use umask() to fetch a process's umask without at the same time changing it. A second call to umask() would then be
needed to restore the umask. The nonatomicity of these two steps provides the potential for races in multithreaded programs.
Since Linux 4.7, the umask of any process can be viewed via the Umask field of /proc/[pid]/status. Inspecting this field in
/proc/self/status allows a process to retrieve its umask without at the same time changing it.
So it seems to be possible to just get the umask-info , but only on some OS versions .
Don't know how relevant the small time window is, in case of hs_err file writing probably not so relevant.
Btw. there is a similar pattern here :
jdk/src/java.prefs/unix/native/libprefs/FileSystemPreferences.c
94 old_umask = umask(0);
95 fd = open(fname, O_WRONLY|O_CREAT, permission);
96 result[1] = errno;
97 umask(old_umask);
Best regards, Matthias
> -----Original Message-----
> From: Thomas Stüfe <thomas.stuefe at gmail.com>
> Sent: Freitag, 9. November 2018 10:08
> To: Baesken, Matthias <matthias.baesken at sap.com>
> Cc: David Holmes <david.holmes at oracle.com>; Langer, Christoph
> <christoph.langer at sap.com>; Volker Simonis <volker.simonis at gmail.com>;
> HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>
> Subject: Re: RFR : 8211326 : add OS user related information to hs_err file
>
> Hi Matthias,
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.2/src/hotspot/os/
> posix/os_posix.hpp.udiff.html
>
> + static void print_user_info(outputStream* st, char* buf, size_t buflen);
>
> You can get rid of buf+buflen too, not needed anymore
> ---
> http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.2/src/hotspot/os/
> posix/os_posix.cpp.udiff.html
>
> In os::Posix::print_user_info(), I would probably now print it all one
> one like, e.g. "uid: 4711, euid: 0, euid: 4711, egid: 4711, umask:
> rwxr--r--" - for brevity. But I leave that up to you.
> ---
>
> The "removing" text does not make sense and can itself be removed :)
>
> --
>
> I spotted a tiny issue which puts the whole print umask into question.
> Appearantly, there is no way for umask() to just return the current
> value of the file creation mask without changing it. So, that is why
> we restore it right afterwards. That is annoying. That leaves a tiny
> tiny time window where a different thread could create a file which
> would then have permissions all zero'd out.
>
> I am not sure how to deal with that. Could you look around whether
> there is another posix API to just look at the umask without changing
> it? Otherwise: does anyone think this is a problem?
>
> --
>
> Sorry, turns out simple things are often more complicated than it
> first appears...
>
> Cheers, Thomas
>
>
>
>
>
>
> On Fri, Nov 9, 2018 at 9:45 AM Baesken, Matthias
> <matthias.baesken at sap.com> wrote:
> >
> > Hi Thomas, thank you for the remarks .
> > I introduced
> >
> > os::Posix::print_umask
> >
> > >
> > > But I remember that we did this in our
> > > port after some deliberation - there is a risk that name resolving may
> > > hang or crash, depending on how exactly user ids are resolved on that
> machine.
> > >
> >
> > ..and removed the getpwuid / getgrgid calls (so we only get the
> numeric ids in the output and not the user names / group names)
> > New webrev :
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.2/
> >
> >
> > Best regards, Matthias
> >
> >
> > > -----Original Message-----
> > > From: Thomas Stüfe <thomas.stuefe at gmail.com>
> > > Sent: Donnerstag, 8. November 2018 21:00
> > > To: Baesken, Matthias <matthias.baesken at sap.com>
> > > Cc: David Holmes <david.holmes at oracle.com>; Langer, Christoph
> > > <christoph.langer at sap.com>; Volker Simonis
> <volker.simonis at gmail.com>;
> > > HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>
> > > Subject: Re: RFR : 8211326 : add OS user related information to hs_err file
> > >
> > > Hi Matthias,
> > >
> > > this looks good in general. Two remarks:
> > >
> > > 1) For convenience reasons, we resolve the user name from its id (and
> > > group name etc) with getpwuid. But I remember that we did this in our
> > > port after some deliberation - there is a risk that name resolving may
> > > hang or crash, depending on how exactly user ids are resolved on that
> > > machine. For our port we decided to go with it, since our VM runs
> > > usually in very controlled environments, but for the OpenJDK I propose
> > > to remove that part from the patch and just print out the numeric
> > > values.
> > >
> > > 2) The part where you print out the mode_t umask as "rwxrwxrwx" is
> > > kind of neat. Maybe we could factor that out as an own function in
> > > os::Posix? e.g. os::Posix::print_umask(mode_t mask) ?
> > >
> > > I leave (2) up to you. We also can do this in a follow up patch.
> > >
> > > Thanks, Thomas
> > >
> > >
> > > On Thu, Nov 8, 2018 at 6:13 PM Baesken, Matthias
> > > <matthias.baesken at sap.com> wrote:
> > > >
> > > > Hello , after the flag "-XX:+-ExtensiveErrorReports" make it into
> jdk/jdk ,
> > > I created another webrev :
> > > >
> > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.1/
> > > >
> > > >
> > > > The user info output is now guarded by ExtensiveErrorReports .
> > > >
> > > > Please review!
> > > >
> > > >
> > > > Thanks, Matthias
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Baesken, Matthias
> > > > > Sent: Dienstag, 9. Oktober 2018 16:55
> > > > > To: 'Thomas Stüfe' <thomas.stuefe at gmail.com>; David Holmes
> > > > > <david.holmes at oracle.com>
> > > > > Cc: Langer, Christoph <christoph.langer at sap.com>; Volker Simonis
> > > > > <volker.simonis at gmail.com>; HotSpot Open Source Developers
> > > <hotspot-
> > > > > dev at openjdk.java.net>
> > > > > Subject: RE: RFR : 8211326 : add OS user related information to hs_err
> file
> > > > >
> > > > > Hi Thomas , thanks for filing the CSR .
> > > > >
> > > > > I am fine with the name proposal "-XX:+-ExtensiveErrorReports",
> but
> > > will
> > > > > wait until it is agreed to use this name .
> > > > > Then I'll post another webrev of 8211326 using the flag .
> > > > >
> > > > > Best regards, Matthias
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Thomas Stüfe <thomas.stuefe at gmail.com>
> > > > > > Sent: Montag, 8. Oktober 2018 12:21
> > > > > > To: David Holmes <david.holmes at oracle.com>
> > > > > > Cc: Baesken, Matthias <matthias.baesken at sap.com>; Langer,
> > > Christoph
> > > > > > <christoph.langer at sap.com>; Volker Simonis
> > > > > <volker.simonis at gmail.com>;
> > > > > > HotSpot Open Source Developers <hotspot-
> dev at openjdk.java.net>
> > > > > > Subject: Re: RFR : 8211326 : add OS user related information to
> hs_err
> > > file
> > > > > >
> > > > > > On Mon, Oct 8, 2018 at 11:50 AM David Holmes
> > > > > <david.holmes at oracle.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Thomas,
> > > > > > >
> > > > > > > On 8/10/2018 7:43 PM, Thomas Stüfe wrote:
> > > > > > > > Hi all,
> > > > > > > >
> > > > > > > > I drafted a CSR:
> > > > > > > >
> > > > > > > > https://bugs.openjdk.java.net/browse/JDK-8211846
> > > > > > > >
> > > > > > > > Not sure what to do now, should I open a discussion thread on a
> > > > > > > > mailing list? Or just wait for the committee to decide? (I'm doing
> > > > > > > > this too rarely..)
> > > > > > >
> > > > > > > I filled in some of the additional info needed. The text needs to be
> > > > > > > completed and the CSR needs to have at least one reviewer
> added to
> > > it.
> > > > > > > Then it can be marked "final" and ready for the committee to
> > > evaluate.
> > > > > > >
> > > > > >
> > > > > > Thanks! I filled out what was missing and will wait for reviewers.
> > > > > >
> > > > > > Thanks, Thomas
> > > > > >
> > > >
More information about the hotspot-dev
mailing list