| Message ID | 743b3ff3-896c-bfc9-e187-6d50da88f103@suse.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | xen-netfront: XSA-403 follow-on | expand |
On Wed, 2022-07-13 at 11:19 +0200, Jan Beulich wrote: > Check the retrieved grant reference first; there's no point trying to > have xennet_move_rx_slot() move invalid data (and further defer > recognition of the issue, likely making diagnosis yet more difficult). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > I question the log message claiming a bad ID (which is how I read its > wording): rx->id isn't involved in determining ref. I don't see what > else to usefully log, though, yet making the message just "Bad rx > response" also doesn't look very useful. For the records, I (mis-)read that log message differently, claiming there is a bad RX response, and specifying the ID of such response, which may or may be not useful to diagnose where/when the problem happens. Cheers, Paolo
--- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -1043,16 +1043,6 @@ static int xennet_get_responses(struct n } for (;;) { - if (unlikely(rx->status < 0 || - rx->offset + rx->status > XEN_PAGE_SIZE)) { - if (net_ratelimit()) - dev_warn(dev, "rx->offset: %u, size: %d\n", - rx->offset, rx->status); - xennet_move_rx_slot(queue, skb, ref); - err = -EINVAL; - goto next; - } - /* * This definitely indicates a bug, either in this driver or in * the backend driver. In future this should flag the bad @@ -1065,6 +1055,16 @@ static int xennet_get_responses(struct n err = -EINVAL; goto next; } + + if (unlikely(rx->status < 0 || + rx->offset + rx->status > XEN_PAGE_SIZE)) { + if (net_ratelimit()) + dev_warn(dev, "rx->offset: %u, size: %d\n", + rx->offset, rx->status); + xennet_move_rx_slot(queue, skb, ref); + err = -EINVAL; + goto next; + } if (!gnttab_end_foreign_access_ref(ref)) { dev_alert(dev,
Check the retrieved grant reference first; there's no point trying to have xennet_move_rx_slot() move invalid data (and further defer recognition of the issue, likely making diagnosis yet more difficult). Signed-off-by: Jan Beulich <jbeulich@suse.com> --- I question the log message claiming a bad ID (which is how I read its wording): rx->id isn't involved in determining ref. I don't see what else to usefully log, though, yet making the message just "Bad rx response" also doesn't look very useful.