[Swan] [foobar.fi #2525] Fw: Valgrin reported issues (and threads) (fwd)

Paul Wouters paul at nohats.ca
Tue Mar 5 21:54:24 EET 2013


On Tue, 5 Mar 2013, D. Hugh Redelmeier wrote:

> Threads are a hairy thing.  And, apparently, we cannot trust whoever
> used them to get it right.  And the compiler apparently doesn't warn
> about this kind of mistake.
>
> Yikes.
>
> | Interestingly crypt() is also not thread safe.
>
> Is crypt being used?  By more than one thread?

Yes, for XAUTH authentication using /etc/ipsec.passwd when xauthby=file

I'll fix it.

> | possible fix for localtime() calls is localtime_r() calls
>
> Actually, a very easy fix.

Yeah, and the same for gettime/gettime_r

> There are several functions in Pluto that are related to logging that
> are not thread safe: they use static buffers.
>
> - enum_show

I meant to lookup the difference with enum_name and enum_show. I'll have
to check where enum_show is used I guess.

> - bitnamesof.
>
> - bitamesofb could have been re-entrant but isn't.  Did I write that?

I don't know :)

>  Doesn't currently matter since the only two uses are not re-entrant.
>
>  But it is wrong in another way since it defines a static buffer for
>  formatting hex that isn't large enough with the new wider type for
>  lset_t.
>
>  It could be rewritten to be re-entrant and not have this bug (I've
>  started).

Thanks!

> - prettypolicy

AFAIK, that is only used in 'ipsec auto --status', not in threads.

> - sparse_val_show
>
> and no doubt more.

We should create a list for developers.

Paul


More information about the Swan mailing list