linux-kernel - Re: [PATCH v2 3/8] watchdog: at91rm9200: use the regmap from mfd

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: <54B1CE58.4040407@roeck-us.net>
Date:	Sat, 10 Jan 2015 17:14:00 -0800
From:	Guenter Roeck <linux@...ck-us.net>
To:	Alexandre Belloni <alexandre.belloni@...e-electrons.com>
CC:	Nicolas Ferre <nicolas.ferre@...el.com>,
	Boris Brezillon <boris.brezillon@...e-electrons.com>,
	Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Lee Jones <lee.jones@...aro.org>,
	Wim Van Sebroeck <wim@...ana.be>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-watchdog@...r.kernel.org
Subject: Re: [PATCH v2 3/8] watchdog: at91rm9200: use the regmap from mfd

On 01/10/2015 10:41 AM, Alexandre Belloni wrote:
> On 09/01/2015 at 16:39:21 -0800, Guenter Roeck wrote :
>> On 01/09/2015 01:51 AM, Alexandre Belloni wrote:
>>>   /* ......................................................................... */
>>> @@ -204,6 +201,7 @@ static struct miscdevice at91wdt_miscdev = {
>>>   static int at91wdt_probe(struct platform_device *pdev)
>>>   {
>>>   	int res;
>>> +	regmap_st = dev_get_drvdata(pdev->dev.parent);
>>>
>>
>> Is it guaranteed that parent is never NULL, and that the parent's
>> drvdata is always set ?
>>
>
> The only way to probe the driver left is to use platform_data. It is
> done from the MFD driver. If you prefer, I can test for NULL here and
> return.
>

I am fine if you are sure about that. The problem though is still the
check for double instantiation. Either the driver can not be instantiated
multiple times, and the check is no longer necessary, or it can, meaning
there must be some other means to instantiate it besides through the
mfd driver. And in that case you don't necessarily know if platform_data
is set.

>> Also, it seems that regmap_st will be overwritten if the device
>> is already open (see code below). That may not be a good idea.
>>
>
> I'm not sure what you meani, pdev->dev.parent and at91wdt_miscdev.parent
> are not the same thing. I didn't touch the code below but I believe
> there is no reason to pass in the probe twice and return -EBUSY.
>

Not sure I understand what my comment has to do with 'parent'.

The fact that the code checks if it has already been instantiated
suggests that the original author of the driver was concerned about
that case. Since you did not remove that check I have to assume that
this can still happen. If so, the second instantiation attempt will
overwrite regmap_st, since that variable is written prior to checking
for the dual instantiation.

Guenter

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