linux-kernel - Re: perf: behavior of poll() changed in 3.18

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150121162108.GD6835@krava.brq.redhat.com>
Date:	Wed, 21 Jan 2015 17:21:08 +0100
From:	Jiri Olsa <jolsa@...hat.com>
To:	Vince Weaver <vincent.weaver@...ne.edu>
Cc:	linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Ingo Molnar <mingo@...nel.org>,
	Paul Mackerras <paulus@...ba.org>
Subject: Re: perf: behavior of poll() changed in 3.18

On Wed, Jan 21, 2015 at 01:06:30AM -0500, Vince Weaver wrote:
> On Tue, 20 Jan 2015, Jiri Olsa wrote:
> 
> > I made this change to get notification that monitored
> > process exited.  We use it in 'perf record' to find out
> > that we have nothing more to monitor and quit.
> > 
> > The logic is to return POLLHUP (via poll syscall) when
> > the monitored process exits for the event.
> > 
> > Could you please share your test code?
> 
> My code does something like this to count the number of overflows
> happening in a child:
> 
> 	/* fds contains one fd, which is a harware event */
> 	/* attached to a child */
> 
> 	while(1) {
>                 result=poll(fds,1,100);
>                 if (result==0) {
>                         waitpid(child,&status,WNOHANG);

		unrelated, but I noticed, that this seems broken.. you shouldn't check
		WIFEXITED(status) without waitpid(... WNOHANG) returning > 0

>                         if (WIFEXITED(status)) break;
>                         if (WIFSIGNALED(status)) {
>                                 printf("Signalled %d!\n",WTERMSIG(status));
>                                 break;
>                         }
>                 }
> 
>                 if (fds[0].revents&POLLIN) count.in++;
>                 if (fds[0].revents&POLLHUP) count.hup++;
>                 if (fds[0].revents&POLLERR) break;
> 	}
> 
> With the change you made this code hangs forever because poll no longer
> returns "0" when the child exits, but instead returns "1" with
> POLLHUP set.

right, once event is HUPed the poll will return POLLHUP in revents

> 
> I admit my code isn't the best out there, but it worked reliably up
> until 3.18 when the change was made.

I think that poll returning POLLHUP for an event which cease to work
due to its process died is correct.

On the other side we broke your code, which is against the policy.

However if we revert this code, we'll loose nice (and standard) way
to check if the event is still valid.. not sure how to handle this.

thoughts?

> 
> Part of why my code doesn't just exit on POLLHUP is because you can
> get that result for reasons other than a process exit (for example,
> if you are using ioctl(PERF_EVENT_IOC_REFRESH)

Nope, this is related to POOL_HUP (notice the '_') which you'll get
accompanied with SIGIO if you setup this.

Before you could get POLLHUP (through poll) only if you'd called
poll on unmapped event.

Now (after commit 179033b3e064) you'll get it also if the process
that the event monitored has died.


jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/