linux-kernel - Re: [PATCHv3 05/16] pps: access pps device by direct pointer

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: <20100805154231.52555130@desktopvm.lvknet>
Date:	Thu, 5 Aug 2010 15:42:31 +0400
From:	Alexander Gordeev <lasaine@....cs.msu.su>
To:	Rodolfo Giometti <giometti@...eenne.com>
Cc:	linux-kernel@...r.kernel.org,
	"Nikita V. Youshchenko" <yoush@...msu.su>,
	linuxpps@...enneenne.com, john stultz <johnstul@...ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tejun Heo <tj@...nel.org>, Joonwoo Park <joonwpark81@...il.com>
Subject: Re: [PATCHv3 05/16] pps: access pps device by direct pointer

В Thu, 5 Aug 2010 11:32:36 +0200
Rodolfo Giometti <giometti@...eenne.com> пишет:

> On Thu, Aug 05, 2010 at 01:06:42AM +0400, Alexander Gordeev wrote:
> > Using device index as a pointer needs some unnecessary work to be done
> > every time the pointer is needed (in irq handler for example).
> > Using a direct pointer is much more easy (and safe as well).
> > 
> > Signed-off-by: Alexander Gordeev <lasaine@....cs.msu.su>
[snip]
> 
> If you remove these functions you can't be sure anymore that nobodies
> may call pps_event() over a non existent device...

[snip]

> By dropping pps_get_source you may be here by a call from (i.e.) a
> serial port driver whose doesn't know if your PPS source is gone or
> not...
> 
> I don't understand how your modifications may resolve this problem.

Well, this can happen only if PPS client module calls pps_event before
calling pps_register_source() or after pps_unregister_source(). This
means that it's broken! If we try to handle/workaround broken clients it
affects performance. So we have to choose what is the priority:
security or performance. My guru told me I shouldn't bother too much
about broken kernel-space code which my code interacts with. If it's
broken it should be fixed. Some assertions enabled by DEBUG define are
enough. For me it makes sense but I don't know what should I check here?

Well I can add something like that to pps_event:

BUG_ON((pps == NULL) || (pps_get_source(pps->id) != pps));

where pps_get_source is:

static inline struct pps_device *pps_get_source(int source)
{
	struct pps_device *pps;
	unsigned long flags;

	spin_lock_irqsave(&pps_idr_lock, flags);
	pps = idr_find(&pps_idr, source);
	spin_unlock_irqrestore(&pps_idr_lock, flags);

	return pps;
}

BTW, while looking at the code to answer your question I've found a
bug: struct pps_device was not kfree'd on device destruction. The fix
will appear soon. Perhaps with an assertion above if you like it.

-- 
  Alexander

Download attachment "signature.asc" of type "application/pgp-signature" (490 bytes)