On Fri, Apr 20, 2012 at 10:11:37AM -0700, Jeff Kirsher wrote: > From: Alexander Duyck <alexander.h.du...@intel.com> > > The fcoe_recv_frame path makes a call to skb_linearize without verifying > that it has actually linearized the frame. This can lead to memory > corruption issues if we use the frame assuming it is 3K, instead of a > smaller 512 or 1K sized frame. > > In order to ensure that the linearize call result is tested I have moved it > into the location that was previously taken in fcoe_rcv which was verifying > we had at least the header. This way instead of breaking things up into > two calls to __pskb_pull_tail we rip the bandage off in one tear and just > take care of all of the linearization in a single call. > > A better fix would be to have the FCoE path handle non-linear sk_buffs, but > I am not confident enough in my understanding of FCoE to rewrite it to > handle it. I will leave that to the FCoE maintainers to handle. > > Signed-off-by: Alexander Duyck <alexander.h.du...@intel.com> > Tested-by: Ross Brattain <ross.b.bratt...@intel.com> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirs...@intel.com> > > --- > drivers/scsi/fcoe/fcoe.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c > index e959960..5d37ce6 100644 > --- a/drivers/scsi/fcoe/fcoe.c > +++ b/drivers/scsi/fcoe/fcoe.c > @@ -1336,11 +1336,12 @@ static int fcoe_rcv(struct sk_buff *skb, struct > net_device *netdev, > } > > /* > - * Check for minimum frame length, and make sure required FCoE > + * Check for minimum frame length, and make sure FCoE frame > * and FC headers are pulled into the linear data area. > + * > + * This is not ideal, and FCoE should learn to handle non-linear frames > */ > - if (unlikely((skb->len < FCOE_MIN_FRAME) || > - !pskb_may_pull(skb, FCOE_HEADER_LEN))) > + if (unlikely((skb->len < FCOE_MIN_FRAME) || skb_linearize(skb))) > goto err; > this moves the call to skb_linearize into softirq context however. Is that safe to do? I can't find any other code point that does this, so I can't say for certain. It looks ok, but I'm not sure.
Also, this is going to increase softirq execution time. FCOE_MIN_FRAME is defined as 38 bytes, so it seems to me that that the pskb_may_pull will likely return very quickly, whereas the the linearize call has lots of potential to take a long time. Might it be better to insead, just reduce fcoe_rcv to a minimum effort routine that parses out the fh_ox_id value from the skb to determine the destination cpu of the frame? Then we can move all the other validation work that fcoe_rcv does up to fcoe_recv_frame, where we run in process context? I like the idea of not having to do two pulls/linearizations, but I think it might be better to do it out of the softirq handler if we can. Regards Neil > skb_set_transport_header(skb, sizeof(struct fcoe_hdr)); > @@ -1676,7 +1677,6 @@ static void fcoe_recv_frame(struct sk_buff *skb) > skb->dev ? skb->dev->name : "<NULL>"); > > port = lport_priv(lport); > - skb_linearize(skb); /* check for skb_is_nonlinear is within > skb_linearize */ > > /* > * Frame length checks and setting up the header pointers > _______________________________________________ > devel mailing list > devel@open-fcoe.org > https://lists.open-fcoe.org/mailman/listinfo/devel > _______________________________________________ devel mailing list devel@open-fcoe.org https://lists.open-fcoe.org/mailman/listinfo/devel
