Bug 76398 - [libc] stdio can lose data in the presence of signals
Summary: [libc] stdio can lose data in the presence of signals
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 6.0-CURRENT
Hardware: Any Any
: Normal Affects Only Me
Assignee: Konstantin Belousov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-01-18 09:50 UTC by Yar Tikhiy
Modified: 2024-01-20 22:11 UTC (History)
6 users (show)

See Also:


Attachments
Corrected Yar' test (3.16 KB, text/plain)
2022-01-23 07:04 UTC, Konstantin Belousov
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yar Tikhiy 2005-01-18 09:50:24 UTC
A signal handler can be installed without the SA_RESTART flag so
that the signal will interrupt certain syscalls.  The application
should be able to restart the interrupted operation by itself if
it installs such signal handlers.  However, stdio isn't ready for
this approach.  An interrupted write call will be treated by stdio
as an unrecoverable error condition and the current buffer contents
will be lost.  This is not dictated by the design, but is due to
the manner stdio has been coded in.  See __sflush() in particular,
which is the major source of the problem.

Fix: 

I'm trying to work one out.
How-To-Repeat: 
Here's a test program demonstrating the bug easily.

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

#include <sys/types.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <err.h>
#include <errno.h>
#include <md5.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

/* #define CHILD_READER */
/* #define USE_PIPE */

#define NDATA	1000
#define	DELAY	2

void  hup(int);
void  setdata(int);
char *getdata(void);

int
main()
{
	char c, digest0[33], digest[33], *p;
	FILE *fp;
	int i, s[2];
	MD5_CTX md5;
	pid_t child;
	struct sigaction sa;

	MD5Init(&md5);
	setdata(NDATA);
	while ((p = getdata()))
		MD5Update(&md5, p, strlen(p));
	p = MD5End(&md5, digest0);
	printf("True digest is %s\n", digest0);

	sa.sa_handler = hup;
	sigemptyset(&sa.sa_mask);
	sa.sa_flags = 0;
	if (sigaction(SIGHUP, &sa, NULL) == -1)
		err(2, "sigaction");

#ifndef USE_PIPE
	if (socketpair(PF_UNIX, SOCK_STREAM, 0, s) < 0)
		err(2, "socketpair");
#else
	if (pipe(s) < 0)
		err(2, "pipe");
#endif

	switch (child = fork()) {
	case -1:
		err(2, "fork");

#ifndef CHILD_READER
	case 0:
#else
	default:
#endif
		if ((fp = fdopen(s[0], "w")) == NULL)
			err(2, "fdopen in writer");
		close(s[1]);
		setdata(NDATA);
		while ((p = getdata()))
			for (; *p; p++)
				if (fputc(*p, fp) == EOF) {
					if (errno == EINTR)
						clearerr(fp);
					else
						err(2, "fputc");
				}
		fclose(fp);
#ifdef CHILD_READER
		waitpid(child, &i, 0);
#endif
		break;

#ifndef CHILD_READER
	default:
#else
	case 0:
#endif
		close(s[0]);
		if ((fp = fdopen(s[1], "r")) == NULL)
			err(2, "fdopen in reader");
		sleep(DELAY);
#ifndef CHILD_READER
		if (kill(child, SIGHUP) == -1)
#else
		if (kill(getppid(), SIGHUP) == -1)
#endif
			err(2, "kill");
		sleep(DELAY);
		MD5Init(&md5);
		while ((i = fgetc(fp)) != EOF) {
			c = i;
			MD5Update(&md5, &c, 1);
		}
		MD5End(&md5, digest);
		printf(" Got digest of %s\n", digest);
		fclose(fp);
		break;
	}
	return (0);
}

void
hup(int signo __unused)
{
}

static int ndata, seq;

void
setdata(int n)
{
	
	ndata = n;
	seq = 0;
}

char *
getdata()
{
	static char databuf[256];

	if (seq >= ndata)
		return (NULL);
	sprintf(databuf, "%08d xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n", seq++);
	return (databuf);
}

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Comment 1 Eugene Grosbein freebsd_committer freebsd_triage 2018-02-02 23:29:07 UTC
Is this problem still relevant?
Comment 2 Piotr Pawel Stefaniak freebsd_committer freebsd_triage 2021-10-09 13:11:59 UTC
I still get this on a stable/13:
$ cc test.c -lmd -o test
$ ./test
True digest is 415f99bc5372a53f20931217a8fc211d
 Got digest of 8376655daf0bdbbca3b7548cec51a2d6
Comment 3 Eugene Grosbein freebsd_committer freebsd_triage 2022-01-21 20:00:27 UTC
Re-open as problem reported to be actual.
CC'ng kib@ who may have something to tell on the topic.
Comment 4 Konstantin Belousov freebsd_committer freebsd_triage 2022-01-23 07:03:39 UTC
See https://reviews.freebsd.org/D34007 at least for the problem demonstrated
by the test.
Comment 5 Konstantin Belousov freebsd_committer freebsd_triage 2022-01-23 07:04:23 UTC
Created attachment 231246 [details]
Corrected Yar' test
Comment 6 commit-hook freebsd_committer freebsd_triage 2022-01-25 15:26:23 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=86a16ada1ea608408cec370171d9f59353e97c77

commit 86a16ada1ea608408cec370171d9f59353e97c77
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2022-01-23 06:52:59 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2022-01-25 15:26:05 +0000

    __sflush(): on write error, if nothing was written, reset FILE state back

    otherwise the data is just dropped.  Check for current position equal to
    the buffer base at the entry of the function; if not equal, setvbuf()
    was done from the write method and it is not our business to override
    the decision.

    PR:     76398
    Reviewed by:    markj
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D34007

 lib/libc/stdio/fflush.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2022-01-27 23:14:01 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=72d5dedfa69fa74801d19dd17c49867882d73d97

commit 72d5dedfa69fa74801d19dd17c49867882d73d97
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2022-01-25 21:56:08 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2022-01-27 23:09:47 +0000

    stdio: add test for 86a16ada1ea608408cec370: fflush() handling of errors

    PR:     76398
    Reviewed by:    emaste, markj
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D34044

 lib/libc/tests/stdio/Makefile           |   2 +
 lib/libc/tests/stdio/eintr_test.c (new) | 143 ++++++++++++++++++++++++++++++++
 2 files changed, 145 insertions(+)
Comment 8 commit-hook freebsd_committer freebsd_triage 2022-02-01 03:30:39 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ec2db06d0db22ae11c1b5414446e3aecd71a93e3

commit ec2db06d0db22ae11c1b5414446e3aecd71a93e3
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2022-01-25 21:56:08 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2022-02-01 03:29:16 +0000

    stdio: add test for 86a16ada1ea608408cec370: fflush() handling of errors

    PR:     76398

    (cherry picked from commit 72d5dedfa69fa74801d19dd17c49867882d73d97)

 lib/libc/tests/stdio/Makefile           |   2 +
 lib/libc/tests/stdio/eintr_test.c (new) | 143 ++++++++++++++++++++++++++++++++
 2 files changed, 145 insertions(+)
Comment 9 commit-hook freebsd_committer freebsd_triage 2022-02-01 03:30:40 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=afa9a1f5ec9974793a8744c55036ef5c4d08903d

commit afa9a1f5ec9974793a8744c55036ef5c4d08903d
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2022-01-23 06:52:59 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2022-02-01 03:29:15 +0000

    __sflush(): on write error, if nothing was written, reset FILE state back

    PR:     76398

    (cherry picked from commit 86a16ada1ea608408cec370171d9f59353e97c77)

 lib/libc/stdio/fflush.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Comment 10 Eugene Grosbein freebsd_committer freebsd_triage 2022-06-27 22:21:54 UTC
Assign the PR to the committer resolving issue.