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  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:	Tue, 9 Aug 2016 20:09:36 -0500
From:	Timur Tabi <>
To:	Florian Fainelli <>,
	Lino Sanfilippo <>,,,,,,,,,,,,,,,
Subject: Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

Florian Fainelli wrote:

> nr_frags can't be bigger than MAX_SKB_FRAGS, hence these checks all
> other drivers do against 1 + MAX_SKB_FRAGS.

Doh, I just realized something.  emac_mac_tx_buf_send() just needs to 
make sure that there's enough room for ONE skb.  For some reason I 
thought it had to make sure there's enough room for multiple SKBs.

Now it makes a lot more sense.  Thank you.

So it looks like a given SKB can occupy 3 + nr_frags descriptors.  So I 
need to change that line to:

	if (emac_tpd_num_free_descs(tx_q) < (MAX_SKB_FRAGS + 3))

Question, some  drivers do <= instead of just <, like this:

	if (ring->free_bds <= (MAX_SKB_FRAGS + 1))

Is it necessary to stop the queue if there exactly enough descriptors to 
hold an SKB?  Shouldn't the above be this instead:

	if (ring->free_bds < (MAX_SKB_FRAGS + 1))

>> However, I'm confused about one thing.  Almost every other driver just
>> sets "netdev->mtu = new_mtu" and does nothing else.  I can't find any
>> other driver that actually stops the RX path, reprograms the hardware,
>> and then restarts the RX path.  I know this is a stupid question, but
>> why is my driver doing that?
> Most drivers allocate RX buffer sizes that are usually bigger than the
> MTU, but would probably silently fail or expose transient behavior once
> the MTU changes to greater than the size pre-defined.

So it looks like the real problem is a race condition between

	adpt->rxbuf_size = new_mtu > EMAC_DEF_RX_BUF_SIZE ?
		ALIGN(max_frame, 8) : EMAC_DEF_RX_BUF_SIZE;


	if (netif_running(netdev))
		return emac_reinit_locked(adpt);

That is, if the interface is running, I set rxbuf_size.  If suddenly I 
receive some packets, then the driver will use the wrong buffer size.

Is there an easy way for me to stop the RX path before I set rxbuf_size? 
  Some netif_xxx function I can call?

Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Powered by blists - more mailing lists