linux-kernel - Re: [PATCH 1/4] hid-multitouch: Auto detection of maxcontacts

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: <20110309131643.GA10925@polaris.bitmath.org>
Date:	Wed, 9 Mar 2011 14:16:44 +0100
From:	"Henrik Rydberg" <rydberg@...omail.se>
To:	Benjamin Tissoires <benjamin.tissoires@...c.fr>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Stephane Chatty <chatty@...-enac.fr>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] hid-multitouch: Auto detection of maxcontacts

> > if (td->mtclass->maxcontacts > td->maxcontacts)
> >
> >> +                     /* check if the maxcontacts is given by the class */
> >> +                     td->maxcontacts = td->mtclass->maxcontacts;
> >> +
> >> +             if (!td->maxcontacts)
> >> +                     td->maxcontacts = MT_CONTACTMAX_DEFAULT;
> >
> > this part can be then dropped
> 
> Well, it works the way you are suggesting. BTW this let the corner
> case where someone adds a device (MT_CLS) that does not send the
> contact max and does not initialize the .maxcontact field.

Yes, the patch changes the current, perfectly reasonable assumption
that maxcontact is set. If that change is removed from the patch, it
makes the logic simpler, without changing the semantics of the current
code.

> >> +     td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
> >> +                             GFP_KERNEL);
> >> +
> >
> > Don't we have a race problem here?  It seems the device is started at
> > this point, so I worry that events will be handled when slots is still
> > NULL.
> 
> I tried again yesterday: if I put this line above the hid_hw_start ->
> kernel oops at first touch.
> The point is that hid_hw_start calls hid_connect that do the actual
> calls to input_mapping and feature_mapping.

Yes, that is clear, but the urbs are running, and as fas as I can see,
irqs can be delivered to mt_event(). Minute time window, testing is
not likely to hit this. Adding a test for completed initialization in
mt_event() would make sure.

Oh, and there is no test for failed memory allocation.

Thanks.
Henrik
--
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/