[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200716163022.GA20679@openwall.com>
Date: Thu, 16 Jul 2020 18:30:22 +0200
From: Solar Designer <solar@...nwall.com>
To: owl-dev@...ts.openwall.com
Subject: Re: [PATCH 0/5] pam_tcb update
On Thu, Jul 16, 2020 at 12:56:07AM +0300, Dmitry V. Levin wrote:
> These -Wpointer-sign warnings look harmless.
> I suppose they are quite old, though.
> I've committed a fix.
The warnings are gone for me. Thanks!
> I also see the following warnings:
> nss.c:111:3: warning: 'readdir_r' is deprecated [-Wdeprecated-declarations]
> tcb_unconvert.c:245:2: warning: ignoring return value of 'chown', declared with attribute warn_unused_result [-Wunused-result]
> tcb_chkpwd.c:58:4: warning: ignoring return value of 'seteuid', declared with attribute warn_unused_result [-Wunused-result]
> tcb_chkpwd.c:61:4: warning: ignoring return value of 'seteuid', declared with attribute warn_unused_result [-Wunused-result]
>
> We probably should use readdir instead of readdir_r, this would fix the
> first warning.
Could we? This is inside _nss_tcb_getspent_r(), which is sort of meant
to be reentrant - but I don't understand what this really means for that
particular function given that it's stateful - each subsequent call is
supposed to return the next entry. Maybe what is meant is only that the
return value buffer is caller-provided, so if different buffers are used
then the previous ones are not overwritten by subsequent calls. Not
that the function is really reentrant. Do you know?
Also, why is readdir_r() deprecated? Is readdir() now reentrant?
> There is not much we could do about -Wunused-result warnings.
> In case of 'chown' it make sense to issue a warning.
Yes, I think we can call perror() just like we do on the previous
chown()'s error, but we don't have to return 1 on this one.
> In case of first 'seteuid' error we could skip the second 'seteuid'
> invocation.
Then what to do if the first one succeeds, but the second one fails?
Luckily, this is a separate program, which at that point is about to
terminate. Also, this whole piece is for NIS (right?), the support for
which we've just dropped from the rest of tcb. Should we possibly drop
this piece, too?
Alexander
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.
