netdev - Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller driver

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:	Mon, 8 Aug 2016 16:12:39 +0200
From:	Andreas Werner <andreas.werner@....de>
To:	Kurt Van Dijck <dev.kurt@...dijck-laurijssen.be>
CC:	Wolfgang Grandegger <wg@...ndegger.com>,
	Andreas Werner <andreas.werner@....de>, <mkl@...gutronix.de>,
	<linux-can@...r.kernel.org>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <davem@...emloft.net>,
	<jthumshirn@...e.de>, <andy@...nerandy.de>,
	<michael.miehling@....de>
Subject: Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller
 driver

On Mon, Aug 08, 2016 at 03:06:33PM +0200, Kurt Van Dijck wrote:
> 
> --- Original message ---
> > Date:	Mon, 8 Aug 2016 14:28:39 +0200
> > From: Wolfgang Grandegger <wg@...ndegger.com>
> > 
> [...]
> > >>>+
> > >>>+	if (!(cf->can_id & CAN_RTR_FLAG)) {
> > >>>+		writel(data[0], &cf_buf->data[0]);
> > >>>+		writel(data[1], &cf_buf->data[1]);
> > >>
> > >>Why do you not check cf->can_dlc here as well. And is the extra copy
> > >>necessary.
> > >>
> > >
> > >Yes, I agree with you. The extra copy could be also avoided.
> > >
> > >>>+
> > >>>+		stats->tx_bytes += cf->can_dlc;
> > >>>+	}
> > >>
> > >>If I look to other drivers, they write the data even in case of RTR.
> > >>
> > >
> > >But why?
> > >
> > >A RTR does not have any data, therefore there is no need to write the data.
> > >Only the length is required as the request size.
> > 
> > Yes; I'm wondering as well.
> > 
> > >
> > >If there is a reason behind writing the data of a RTR frame, I can
> > >change that, but for now there is no reason.
> > 
> > Yep.
> 
> I _think_ that copying the data without checking the RTR bit clearly
> avoids a condition and might produce faster code on some machines.
> In any case, it reads easier.
> I'm not sure how that interacts with caches etc etc.
> 
> On the other hand, giving unused data is a bad habit that may reveal
> security information on some places, so better avoid it.
> 
> Kurt

Hi Kurt,
thanks for your comment.

In my opinion, I really prever to NOT copying such data if the RTR flag ist set.

Regards
Andy