linux-kernel - Re: [PATCH 4/18] 2.6.17.9 perfmon2 patch for review: new system calls support

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: <20060829102627.1fee475b.akpm@osdl.org>
Date:	Tue, 29 Aug 2006 10:26:27 -0700
From:	Andrew Morton <akpm@...l.org>
To:	eranian@....hp.com
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/18] 2.6.17.9 perfmon2 patch for review: new system
 calls support

On Tue, 29 Aug 2006 09:59:57 -0700
Stephane Eranian <eranian@....hp.com> wrote:

> > > +
> > > +		PFM_DBG("going wait_inactive for [%d] state=%ld flags=0x%lx",
> > > +			task->pid,
> > > +			task->state,
> > > +			local_flags);
> > > +
> > > +		spin_unlock_irqrestore(&ctx->lock, local_flags);
> > > +
> > > +		wait_task_inactive(task);
> > > +
> > > +		spin_lock_irqsave(&ctx->lock, new_flags);
> > 
> > This sort of thing..
> 
> We need to wait until the task is effectively off the CPU, i.e., with its
> machine state (incl PMU) saved. When we come back we re-run the series of tests.
> This applies only to per-thread, therefore it is not affected by smp_processor_id().
> 

Generally, if a reviewer asks a question and the developer answers that
question, this is a sign that a code comment is needed.  Because others are
likely to ask themselves the same question ;)

> 
> ..
>
> > 
> > When copying a struct from kernel to userspace we must beware of
> > compiler-inserted padding.  Because that can cause the kernel to leak
> > a few bytes of uninitialised kernel memory.
> 
> We are copying out exactly the same amount of data that was passed in.
> 
> Are you suggesting that copy_from/copy_to may copy more data?

No, that's OK.  I was pointing out the problem in situations like this:

struct foo {
	u32 a;
	u64 b;
};

{
	struct foo f;

	f.a = 0;
	f.b = 0;
	copy_to_user(p, &f, sizeof(f));
}

which exposes kernel memory.  As you appear to be confident that the
perfmon code can't do this, all is OK.


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