[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130302232957.GR6181@port70.net>
Date: Sun, 3 Mar 2013 00:29:57 +0100
From: Szabolcs Nagy <nsz@...t70.net>
To: musl@...ts.openwall.com
Subject: Re: Fix for fields in utmp
* Rich Felker <dalias@...ifal.cx> [2013-02-26 18:33:42 -0500]:
> On Tue, Feb 26, 2013 at 09:21:12PM +0100, Szabolcs Nagy wrote:
> > * Ivan Kanakarakis <ivan.kanak@...il.com> [2013-02-26 12:34:43 +0200]:
> > > Linux suicidemachine 3.7.5-1-ARCH #1 SMP PREEMPT Mon Jan 28 10:03:32
> > > CET 2013 x86_64 GNU/Linux
> > > ----------------------------------
> > > --- data/glibc.sizeof 2013-02-26 12:27:48.112569344 +0200
> > > +++ data/musl.sizeof 2013-02-26 12:27:48.119236080 +0200
> > > @@ -90,3 +90,3 @@
> > > float 4
> > > -float_t 4
> > > +float_t 8
> >
> > looks like musl does not respect FLT_EVAL_METHOD on x86_64
> > this should be fixed
>
> I just changed this to 4, as it should be. Or does x86_64 support
> changing the FLT_EVAL_METHOD, in which case we'd need to treat it like
> on i386..
i dont know, so current solution is ok
> > > -int_fast16_t 8
> > > -int_fast32_t 8
> > > +int_fast16_t 4
> > > +int_fast32_t 4
> >
> > these diffs may cause problems
>
> These are actually omitted from the ABI documents if I remember
ok then keep it as is
> > > -jmp_buf 200
> > > +jmp_buf 64
>
> This is because our jmp_buf does not contain sigset_t. However due to
> ABI issues for C++ (glibc's jmp_buf and sigjmp_buf sharing the same
> struct tag) we might want to make them the same anyway. There's an
> existing thread somewhere on the topic.
ok
> > > -regex_t 64
> > > +regex_t 56
>
> I think this should be fixed to match.
glibc's definition is something like:
typedef struct {
char *__x1;
long __x2;
long __x3;
long __x4;
char *__x5;
char *__x6;
size_t re_nsub;
char __x7;
} regex_t;
musl's definition (only re_nsub and __opaque are used):
typedef struct {
size_t re_nsub;
void *__opaque, *__padding[4];
size_t __nsub2;
} regex_t;
> > > -regmatch_t 8
> > > -regoff_t 4
> > > +regmatch_t 16
> > > +regoff_t 8
>
> These are a bug in glibc; their types do not conform to the
> requirements imposed on regex.h and regexec().
so regex abi wont be compatible
> > > -siginfo_t 128
> > > +siginfo_t 136
>
> This is probably a mistake, maybe in the padding..?
yes, the si_fields union is 8byte aligned
glibc's definition (there are some bits left out here and
i think their definition is incorrectly 136 bytes on x32):
struct siginfo {
int si_signo;
int si_errno;
int si_code;
union {
#if 64bit
int _pad[128/sizeof(int) - 4];
#elif 32bit
int _pad[128/sizeof(int) - 3];
#endif
/* ... */
} _sifields;
} siginfo_t;
musl's definition:
struct __siginfo {
int si_signo, si_errno, si_code;
union {
char __pad[128 - 3*sizeof(int)];
/* ... */
} __si_fields;
};
> > > -struct arpd_request 40
> > > +struct arpd_request 28
>
> No idea.
i think this is not used anymore
(it seems there used to be some ioctl command a
userspace arpd might used on linux but the git history
does not go back far enough to tell exactly when it
was dropped and even then it was only used by said
arpd)
glibc's definition:
struct arpd_request {
unsigned short int req;
u_int32_t ip;
unsigned long int dev;
unsigned long int stamp;
unsigned long int updated;
unsigned char ha[7];
};
musl's definition:
struct arpd_request {
uint16_t req;
uint32_t ip;
uint32_t dev;
uint32_t stamp;
uint32_t updated;
uint8_t ha[7];
};
> > > -struct crypt_data 131232
> > > +struct crypt_data 260
>
> This is intentional. glibc's crypt_data is uselessly large and does
> not fit on thread stacks. The only useful part of crypt_data is a
> buffer for storing the resulting hash.
ok
> > > -struct lastlog 292
> > > +struct lastlog 296
>
> Probably needs to be checked.
glibc's definition:
(using the same hack for time_t as in utmpx)
struct lastlog {
int32_t ll_time;
char ll_line[32];
char ll_host[256];
};
musl's definition:
struct lastlog {
time_t ll_time;
char ll_line[32];
char ll_host[256];
};
> > > -struct ntptimeval 72
> > > +struct ntptimeval 32
>
> Should we change this? Probably.
musl does not have ntp_gettime so it probably
should not define this struct
(it is a syscall in bsds and various other unices
with a struct ntptimeval* argument, glibc emulates
this with the linux adjtimex syscall, but probably
this should be done in ntpdate which already has
configure checks for ntp_gettime and struct ntptimeval
and implements its own when they are not available)
(the sizeof ntptimeval has changed and it is
only 72 byte when the NTP_API macro is 4, musl's
definition is the NTP_API 3)
> > > -struct rusage 144
> > > -struct sched_param 4
> > > +struct rusage 272
> > > +struct sched_param 48
>
> Not sure about rusage. For sched_param, we support the extra fields
> for the sporadic server option even though we don't support it (and
> Linux probably never will).
musl's rusage (the difference is the __reserved part):
struct rusage
{
struct timeval ru_utime;
struct timeval ru_stime;
/* linux extentions, but useful */
long ru_maxrss;
long ru_ixrss;
long ru_idrss;
long ru_isrss;
long ru_minflt;
long ru_majflt;
long ru_nswap;
long ru_inblock;
long ru_oublock;
long ru_msgsnd;
long ru_msgrcv;
long ru_nsignals;
long ru_nvcsw;
long ru_nivcsw;
/* room for more... */
long __reserved[16];
};
> > > -struct sockaddr_storage 128
> > > +struct sockaddr_storage 136
>
> Probably wrong.
wrong padding
glibc's definition:
struct sockaddr_storage
{
sa_family_t ss_family;
unsigned long int __ss_align;
char __ss_padding[(128 - (2 * sizeof (unsigned long int)))];
};
musl's definition:
struct sockaddr_storage
{
sa_family_t ss_family;
union {
long long __align;
char __padding[126];
} __padding;
};
> > > -struct sysinfo 112
> > > +struct sysinfo 368
>
> Just extra padding for growth. Maybe it should be changed.
> > > -struct utmpx 384
> > > +struct utmpx 400
> >
> > the comment in glibc sysdeps/gnu/bits/utmpx.h is
> >
> > /* The fields ut_session and ut_tv must be the same size when compiled
> > 32- and 64-bit. This allows files and shared memory to be shared
> > between 32- and 64-bit applications. */
>
> This is idiotic and breaks correct applications that access the ut_tv
> member as ut_tv rather than just accessing its members, since ut_tv
> now has the wrong type and cannot be assigned to/from timeval structs
> and cannot have its address passed to functions that expect timeval
> structs.
>
> Since utmp is not supported anyway, I think it's better to have the
> right types to avoid breaking correct code; the data will never be
> written or read anyway; it just goes to the bitbucket so layout does
> not matter so much.
ok
> > > -uint_fast16_t 8
> > > -uint_fast32_t 8
> > > +uint_fast16_t 4
> > > +uint_fast32_t 4
>
> See above.
> > > -wctrans_t 8
> > > -wctype_t 8
> > > +wctrans_t 4
> > > +wctype_t 4
> >
> > it seems wctype_t is unconditionally long in glibc
>
> As it should be. This is a bug in musl. Being an opaque type that
> could (and often should) be implemented as a pointer, wctype_t should
> always be pointer-sized.
then this should be fixed
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.
