linux-kernel - Re: [PATCH] Fix kunmap() argument in sg_miter

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] [day] [month] [year] [list]

Message-ID: <20081118082715.GH17838@elte.hu>
Date:	Tue, 18 Nov 2008 09:27:15 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Jens Axboe <jens.axboe@...cle.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Jeremy Fitzhardinge <jeremy@...p.org>,
	Tejun Heo <htejun@...il.com>,
	Arjan van de Ven <arjan@...radead.org>,
	Hugh Dickins <hugh@...itas.com>, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org
Subject: Re: [PATCH] Fix kunmap() argument in sg_miter_stop


* Jens Axboe <jens.axboe@...cle.com> wrote:

> On Mon, Nov 17 2008, Linus Torvalds wrote:
> > 
> > 
> > On Mon, 17 Nov 2008, Jens Axboe wrote:
> > > 
> > > Any opinions on the kunmap/kunmap_atomic pointer checking? It's a bit
> > > ugly that we have to enforce a void * rule for kunmap_atomic(),
> > 
> > I don't think that's a "bit ugly". I think it's unacceptable.
> > 
> > Making sure we pass in "struct page" to kunmap() sounds good, but the 
> > kunmap_atomic() part just sounds insane.
> 
> It's been the primary source of bugs that I have seen. The xen and 
> sg iter bug were kunmap() variants though, but otherwise I've mostly 
> seen the opposite. But it is ugly, no doubt about it. I can't think 
> of a better way to attempt to warn about it though, so if you really 
> dislike it I'll just drop the _atomic() bits.

The main ugliness comes from the tons of void * type casts that the 
kunmap_atomic() type check forces. Type casts are just as dangerous 
(and ugly) as type mismatches. (more dangerous in fact)

Perhaps we could try an opt-in 'type filter' approach instead. See 
kernel/tracing/trace.h's trace_assign_type()'s type checking magic for 
an example of how to do it.

( but it's a bit tricky here because we want to filter void * from
  struct page * - i'm not sure gcc will recognize them as incompatible
  types. )

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