bin/75258: [patch] dd(1) has not async signal safe interrupt handlers

[Oleg V. Nauman]

On Fri, Dec 24, 2004 at 02:22:19AM +1100, Bruce Evans wrote:
> On Sun, 19 Dec 2004, Oleg V. Nauman wrote:
> 
> >  On Sun, Dec 19, 2004 at 06:11:06PM +0300, Maxim Konovalov wrote:
> >  > [...]
> >  > > >Description:
> >  > > 	dd(1) uses not safe interrupt handlers, they may leads to
> >  > > strange problems with dd
> >  >=20
> >  > Are you sure?  Do you have a testcase?
> >
> >  	No, sorry.
> 
> However, the patch in the PR would cause strange problems like breaking
> termination of dd when the input is from certain slow devices, e.g.,
> terminals.  Input is normally restarted after a signal, so just setting
> a flag in the signal handler normally doesn't work.  Most "fixes" for
> unsafe signal handling get this wrong.  To get this right, something
> like the following must be done:
> - use sigaction() without SA_RESTART instead of signal(), or use
>   siginterrupt() to turn off SA_RESTART.  I've only seen this done
>   right for fixing unsafe signal handling in ping(8)  I've seen it
>   done wrong in top(1) (try hitting ^C in command mode in top).
> - handle the resulting EINTR errors from all syscalls that may now
>   fail with this error instead of being restarted.  This is difficult
>   or impossible in large programs.  If the signal is checked for and
>   acted on after every EINTR, then you have the same problem as acting
>   on it directly in signal handlers (the syscall might be in a context
>   that doesn't or shouldn't know if it is safe to act), plus
>   maintainence problems for checking and acting all over.  If the
>   acton is delayed, then the program must somehow be structured to
>   ensure that control is passed up to a level that can safely act;
>   in particular, no more syscalls that might block can be made.  I
>   suppose the latter could be implemented simply but laboriously by
>   wrapping all syscalls with
>   { if (signalflag) { errno = EINTR; return (-1); } else { normalcall(); }.

	Thank you for your feedback. It looks like I'm could write much more
better patch :)

> 
> >  >=20
> >  > > >How-To-Repeat:
> >  > >
> >  > > 	man 2 sigaction
> >  >=20
> >  > Well, stdio(3) is not signal-safe in general but it seems for me
> >  > summary() does not manipulate with the internal state of any file
> >  > descriptors (it uses write(2)) and should be safe.
> >
> >  	But s*printf() family uses malloc(3) for his internal purposes,
> >  and there is no any reasons for things like memory allocations in
> >  the signal handler, I think.
> 
> s*printf() needs to be signal-safe in practice.  It is careful to use
> only local storage for this reason.  Only asprintf() uses malloc().

	It looks like snprintf() calls __vfprintf(), which calls __wcsconv()
and __find_arguments() (/usr/src/lib/libc/stdio/vfprintf.c on my
RELENG_5 system)
These functions directly calls malloc() (without counts how many
calls indirectly)

> 
> summary() is obviously attempting to be signal-safe (even in rev.1.1).
> It avoids using printf() but depends on snprintf() being signal safe
> since the formatting would be too hard otherwise.
> 
> Bruce

-- 
NO37-RIPE
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 292 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-bugs/attachments/20041228/110668d1/attachment.bin