Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120306013028.GY184@brightrain.aerifal.cx>
Date: Mon, 5 Mar 2012 20:30:28 -0500
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: Re: utmpx support

On Sun, Mar 04, 2012 at 08:10:36PM +0100, finkler wrote:
> void endutxent(void)
> {
>   memset(&ut, 0, sizeof ut);

These memsets don't seem to be doing anything necessary.

> struct utmpx *getutxent(void)
> {
> 	if (f == NULL) {
>     f = fopen(_PATH_UTMPX, "a+");

Append mode is definitely not correct if you want to be able to
overwrite existing entries.

> struct utmpx *pututxline(const struct utmpx *utmpx)
> {
>   setutxent();
>   if (getutxid(utmpx)) fseek(f, -(sizeof ut), SEEK_CUR);

I'm confused about the intent of this code.. Presumably it's supposed
to modify the entry it just found (relying on getutxid leaving the
file at the right position), but that's not going to work because the
file is opened in append mode.

Also, -(sizeof ut) is not a valid offset; it's almost SIZE_MAX. If you
were using fseeko, this would result in a gigantic offset (rather than
a negative one) after the conversion; with fseek (which takes a long),
it should convert back to the desired negative value, but for safety
you should cast to a signed type *before* negating the result of
sizeof.

There's also the issue that there's absolutely no locking on updates,
so if multiple apps try to add/update utmp entries at the same time,
the file would be corrupted. This could be fixed by using an fcntl
lock on fileno(f) for the duration of the function.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.