linux-kernel - Re: [PATCH 3/3] signalfd: add ability to read siginfo-s without dequeuing signals (v2)

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: <20130209182239.GA9947@redhat.com>
Date:	Sat, 9 Feb 2013 19:22:39 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	"Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>
Cc:	Andrey Wagin <avagin@...il.com>, linux-kernel@...r.kernel.org,
	criu@...nvz.org, linux-fsdevel@...r.kernel.org,
	linux-api@...r.kernel.org,
	Alexander Viro <viro@...iv.linux.org.uk>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	David Howells <dhowells@...hat.com>,
	Dave Jones <davej@...hat.com>,
	Pavel Emelyanov <xemul@...allels.com>,
	Cyrill Gorcunov <gorcunov@...nvz.org>
Subject: Re: [PATCH 3/3] signalfd: add ability to read siginfo-s without
	dequeuing signals (v2)

On 02/08, Michael Kerrisk (man-pages) wrote:
>
> On Fri, Feb 8, 2013 at 8:10 PM, Oleg Nesterov <oleg@...hat.com> wrote:
> >
> > Well. I do not know. Up to you and Michael.
> >
> > But honestly, I can't say this all looks really nice. And why do we
> > need SIGNALFD_PEEK then?
>
> It surely is no beauty. The hope is at least to make it less ugly than it was.

This is subjective, but I am not sure about "less" ;) Yes, we avoid the
magic offsets, but we add SFD_SHARED/PER_THREAD which need to change
dequeue_signal plus other complications. And for what?

> > Seriously, perhaps we should simply add signalfd_fops->ioctl() for PEEK.
> > Or add PTRACE_{PEEK,POKE}_SIGNAL which looks even logical and useful...
> > And much simpler/straightforward.
> >
> > But I am not going to argue.
>
> I suppose I had wondered along similar lines, but in a slightly
> different direction: would the use of a /proc interface to get the
> queued signals make some sense?

(Can't resist sorry... yes we need /proc/pid/cr or /dev/cr or whatever
 which dumps almost everything c/r needs without need to add a lot of
 cr code everywhere).

Perhaps, but I am not sure about the textual representation.

And to me, the best solution is the simplest solution. Please look
at the patch below. It is trivial. And we can also drop the SFD_RAW
patch in -mm.

Oleg.

--- x/kernel/ptrace.c
+++ x/kernel/ptrace.c
@@ -618,6 +618,35 @@ static int ptrace_setsiginfo(struct task
 	return error;
 }
 
+static int ptrace_peek_signal(struct task_struct *child,
+				unsigned long addr, siginfo_t __user *uinfo)
+{
+	siginfo_t info;
+	struct sigpending *pending;
+	int ret = -ESOMETHING;
+
+	pending = &child->pending;
+	if (addr & PTRACE_PEEK_SHARED) {
+		addr &= ~PTRACE_PEEK_SHARED;
+		pending = &child->signal->shared_pending;
+	}
+
+	spin_lock_irq(&child->sighand->siglock);
+	list_for_each_entry(q, &pending->list, list) {
+		if (!addr--) {
+			copy_siginfo(info, &q->info);
+			ret = 0;
+			break;
+		}
+	}
+	spin_lock_irq(&child->sighand->siglock);
+
+	if (!ret)
+		ret = copy_siginfo_to_user(uinfo, info);
+	if (!ret)
+		ret = __put_user(info, si_code);
+	return ret;
+}
 
 #ifdef PTRACE_SINGLESTEP
 #define is_singlestep(request)		((request) == PTRACE_SINGLESTEP)
@@ -742,6 +771,10 @@ int ptrace_request(struct task_struct *c
 		ret = put_user(child->ptrace_message, datalp);
 		break;
 
+	case PTRACE_PEEKSIGNAL:
+		ret = ptrace_peek_signal(child, addr, datavp);
+		break;
+
 	case PTRACE_GETSIGINFO:
 		ret = ptrace_getsiginfo(child, &siginfo);
 		if (!ret)

--
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/