linux-kernel - RE: [PATCH V1] rtc: da9063: access ordering error during RTC interrupt system power on

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]

Date:	Thu, 17 Dec 2015 11:37:06 +0000
From:	"Opensource [Steve Twiss]" <stwiss.opensource@...semi.com>
To:	Alexandre Belloni <alexandre.belloni@...e-electrons.com>
CC:	Alessandro Zummo <a.zummo@...ertech.it>,
	LINUXKERNEL <linux-kernel@...r.kernel.org>,
	RTC-LINUX <rtc-linux@...glegroups.com>,
	"David Dajun Chen" <david.chen@...semi.com>,
	Support Opensource <Support.Opensource@...semi.com>
Subject: RE: [PATCH V1] rtc: da9063: access ordering error during RTC
 interrupt system power on

On 16 December 2015 23:47 Alexandre Belloni wrote:
> Subject: Re: [PATCH V1] rtc: da9063: access ordering error during RTC interrupt system power on
> 
> This seems mostly fine, however ...

Hi Alexandre,
Thanks for reviewing this.

> On 08/12/2015 at 16:28:39 +0000, Steve Twiss wrote :
> >  	irq_alarm = platform_get_irq_byname(pdev, "ALARM");
> >  	ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
> >  					da9063_alarm_event,
> >  					IRQF_TRIGGER_LOW |
> IRQF_ONESHOT,
> >  					"ALARM", rtc);
> > -	if (ret) {
> > +	if (ret)
> >  		dev_err(&pdev->dev, "Failed to request ALARM IRQ %d:
> %d\n",
> >  			irq_alarm, ret);
> > -		return ret;
> > -	}
> > -
> 
> ... now that requesting the interrupt is optional, you probably want to
> prevent userspace from thinking it will get an interrupt after setting
> the alarm by returning -EINVAL in da9063_rtc_read_alarm() and
> da9063_rtc_set_alarm() in that case.
> 

.. I'm not quite certain I understand.
Does the patch looks worse that it is?
This part,

+	if (ret)
		dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n",
			irq_alarm, ret);
 -		return ret;

looks like it has erased the return ret,

> 
> > -	rtc->rtc_dev = devm_rtc_device_register(&pdev->dev,
> DA9063_DRVNAME_RTC,
> > -					   &da9063_rtc_ops, THIS_MODULE);
> > -	if (IS_ERR(rtc->rtc_dev))
> > -		return PTR_ERR(rtc->rtc_dev);
> >
> > -	da9063_data_to_tm(data, &rtc->alarm_time, rtc);
> > -	rtc->rtc_sync = false;
> >  	return ret;

But it does exist at the end of the patch.
So there will still be an error sent if the IRQ is not requested properly.
Is this what you meant in your previous e-mail?

Regards,
Stephen
--
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/