waitid fallout

waitid fallout

[Richard Henderson]

Is an *entire* struct rusage required in siginfo_t for sigchld?

It's much bigger than the 24 (!) words of padding we had reserved
for expansion in siginfo_t.

Which means that the size of siginfo_t changed, which means that
any existing "sigwaitinfo ({ SIGCHLD }, &info)" will now corrupt
user memory.  Perhaps we think no one does this, but we should
at least be aware of the ABI change.

Alpha is not yet using a VDSO for the signal frame unwind info, so
I noticed that all of my signal cancelation tests are now failing,
because the layout of the signal frame changed.  Perhaps I'll take
this opportunity to update, but it does require a rather coordinated
kernel + libc upgrade in order to keep things working smoothly...


r~

Re: waitid fallout

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1169 bytes --]

On Tue, 14 Sep 2004 20:39:03 -0700 Richard Henderson <rth@twiddle.net> wrote:
>
> Is an *entire* struct rusage required in siginfo_t for sigchld?
> 
> It's much bigger than the 24 (!) words of padding we had reserved
> for expansion in siginfo_t.

On 64bit archs, at least.  This means that we break the ABI ...

> Which means that the size of siginfo_t changed, which means that
> any existing "sigwaitinfo ({ SIGCHLD }, &info)" will now corrupt
> user memory.  Perhaps we think no one does this, but we should
> at least be aware of the ABI change.
> 
> Alpha is not yet using a VDSO for the signal frame unwind info, so
> I noticed that all of my signal cancelation tests are now failing,
> because the layout of the signal frame changed.  Perhaps I'll take
> this opportunity to update, but it does require a rather coordinated
> kernel + libc upgrade in order to keep things working smoothly...

We cannot do this even with coordination, as some of the 64bit archs
ABIs have been Standardised in the LSB.

Unless, of course, we revise the ABI Specs as well ...
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: waitid fallout

[Roland McGrath]

> On Tue, 14 Sep 2004 20:39:03 -0700 Richard Henderson <rth@twiddle.net> wrote:
> >
> > Is an *entire* struct rusage required in siginfo_t for sigchld?
> > 
> > It's much bigger than the 24 (!) words of padding we had reserved
> > for expansion in siginfo_t.

Ah, I only ever looked at the sizes on i386, where there was plenty of
room.  As you can see in my posts of the waitid patches, the si_rusage
addition is not required by POSIX.  I chose to add it so that the waitid
call would subsume all functionality of wait4 et al and suffice as the sole
call in the wait family that a new architecture should be obliged to
provide in its system call table.  If we were to take it back out, then it
would make more sense to have the system call be like SGI's waitsys that
takes the waitid args plus a struct rusage *.  

Re: waitid fallout

[Richard Henderson]

On Tue, Sep 14, 2004 at 10:34:40PM -0700, Roland McGrath wrote:
> Ah, I only ever looked at the sizes on i386, where there was plenty of
> room.  As you can see in my posts of the waitid patches, the si_rusage
> addition is not required by POSIX.

I eventually found them (I heart google).  This idea makes a lot of
sense, particularly if rusage had fit everywhere.

> If we were to take it back out, then it would make more sense to have
> the system call be like SGI's waitsys that takes the waitid args plus
> a struct rusage *.  

This solution would make me happy.


r~

Re: waitid fallout

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

On Mittwoch, 15. September 2004 07:34, Roland McGrath wrote:

> If we were to take it back out, then it
> would make more sense to have the system call be like SGI's waitsys that
> takes the waitid args plus a struct rusage *.  

I just noticed it's changed in BK now. However, the respective
changes in the compat_sys_waitid() code and include/linux/syscalls.h
are still missing.

Also, why did you put the compat code in arch/* and not in
kernel/compat.c? Converting siginfo_t should probably remain
architecture specific, but the rest should be the same everywhere.

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: waitid fallout

[Roland McGrath]

> Also, why did you put the compat code in arch/* and not in
> kernel/compat.c? Converting siginfo_t should probably remain
> architecture specific, but the rest should be the same everywhere.

There is no existing shared compat code handling siginfo_t at all.  
I followed the model of existing interfaces that use it like
sys32_rt_sigtimedwait.  I'm not sure that all arch's provide the 32-bit
siginfo_t type or call it the same thing.  I haven't seen other such cases
where the generic kernel/compat.c code refers to types defined in
arch-specific code.


Thanks,
Roland

Re: waitid fallout

[Richard Henderson]

On Tue, Sep 14, 2004 at 08:39:03PM -0700, Richard Henderson wrote:
> Is an *entire* struct rusage required in siginfo_t for sigchld?

FWIW, I don't see anything in 

  http://www.opengroup.org/onlinepubs/009695399/functions/waitid.html
  http://www.opengroup.org/onlinepubs/009695399/basedefs/signal.h.html

that mentions rusage at all.  From whence does this feature arise?


r~

Re: waitid fallout

[Andi Kleen]

On Tue, Sep 14, 2004 at 08:39:03PM -0700, Richard Henderson wrote:
> Is an *entire* struct rusage required in siginfo_t for sigchld?
> 
> It's much bigger than the 24 (!) words of padding we had reserved
> for expansion in siginfo_t.
> 
> Which means that the size of siginfo_t changed, which means that
> any existing "sigwaitinfo ({ SIGCHLD }, &info)" will now corrupt
> user memory.  Perhaps we think no one does this, but we should
> at least be aware of the ABI change.

Oops. I think this broke x86-64 too.

I don't want to change the signal ABI at all there and would prefer
if this change was reverted.

Thanks,
-Andi