Message ID | 2045708.ru9COLib4d@al |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
Peter Wu <lekensteyn@gmail.com> : [...] > Checklist: > 1. super large regs->len: won't be greater than R8169_REGS_SIZE (256) > 2. regs->len == 0: 0 < 0 is false, nothing is copied > 3. regs->len is 1, 2 or 3: i = 0, at most 3 bytes will be copied > 4. regs->len is 4, i < 4 - 4, skip loop, 0 < regs->len, copy 4 > 5. regs->len is 5, i < 5 - 4, copy; 4 < regs->len, copy 1 Spartan implementation: u32 __iomem *ioaddr = tp->mmio_addr; u32 *dw = p; int i; ... for (i = regs->len; i > 0; i -= 4) memcpy_fromio(dw++, ioaddr++, min(i, 4)); or (min() verges on gluttony): for (i = regs->len; i >= 4; i -= 4) memcpy_fromio(dw++, ioaddr++, 4); if (i > 0) memcpy_fromio(dw, ioaddr, i);
On Wed, 2013-08-14 at 23:31 +0200, Peter Wu wrote: > On Wednesday 14 August 2013 21:58:29 Francois Romieu wrote: > > > - memcpy_fromio(p, tp->mmio_addr, regs->len); > > > + if (regs->len >= 4) { > > > + for (i = 0; i < regs->len - 4; i += 4) > > > + memcpy_fromio(bytes + i, tp->mmio_addr + i, 4); > > > + } > > > + if (i < regs->len) > > > > Comparison with random stack stuff when regs->len < 4. :o/ > > Right, let's rm $OLD_PATCH and consider this one. > > Checklist: > 1. super large regs->len: won't be greater than R8169_REGS_SIZE (256) > 2. regs->len == 0: 0 < 0 is false, nothing is copied > 3. regs->len is 1, 2 or 3: i = 0, at most 3 bytes will be copied > 4. regs->len is 4, i < 4 - 4, skip loop, 0 < regs->len, copy 4 > 5. regs->len is 5, i < 5 - 4, copy; 4 < regs->len, copy 1 [...] The kernel buffer size is max(regs->len, dev->ethtool_ops->get_regs_len()). So you can safely ignore regs->len and always read all your registers. Ben.
On Thursday 15 August 2013 22:33:15 Ben Hutchings wrote: > The kernel buffer size is max(regs->len, > dev->ethtool_ops->get_regs_len()). So you can safely ignore regs->len > and always read all your registers. I see, that is something for a different patch though. While I am at it, I checked all users of get_regs_len in drivers/net and found only one other user that also checked the length. Patch will follow shortly. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c index b5eb419..19524c0 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -1897,12 +1897,19 @@ static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *p) { struct rtl8169_private *tp = netdev_priv(dev); + char *bytes = p; + int i = 0; if (regs->len > R8169_REGS_SIZE) regs->len = R8169_REGS_SIZE; rtl_lock_work(tp); - memcpy_fromio(p, tp->mmio_addr, regs->len); + if (regs->len >= 4) { + for (; i < regs->len - 4; i += 4) + memcpy_fromio(bytes + i, tp->mmio_addr + i, 4); + } + if (i < regs->len) + memcpy_fromio(bytes + i, tp->mmio_addr + i, regs->len - i); rtl_unlock_work(tp); }