On 10/23/10 12:20 AM, Hillf Danton wrote: > On Sat, Oct 23, 2010 at 2:53 AM, Joe Eykholt<[email protected]> wrote: >> On 10/22/10 8:04 AM, Hillf Danton wrote: >>> There seems memset could be removed since the frame, returned by >>> fc_frame_alloc_fill with the filled bytes not zeroed, could reach its >>> receiver on the other end. >> >> That's why the memset is there, just to make sure the padding is zero. >> >> Otherwise, up to 3 bytes of uninitialized data could be sent. >> That seems like a very minor security problem, but still would be wrong >> to leak any uninitialized data, and it just seems nicer if the pad bytes >> are zero, even though they can be anything at all. >> >> What may be confusing to some readers is that we use skb_trim() to >> trim the length to not include the pad, but later in fc_exch_setup_hdr() >> we add the pad back (without zeroing it at that point). >> >> So, this patch should not be applied. >> > > The padded is not zeroed. > The zeroed lies in the range of payload whose len is payload_len.
Not true. The padding starts at fc_hdr + payload len, and goes for fill bytes. >>> >>> Signed-off-by: Hillf Danton<[email protected]> >>> --- >>> >>> --- a/drivers/scsi/libfc/fc_frame.c 2010-09-13 07:07:38.000000000 +0800 >>> +++ b/drivers/scsi/libfc/fc_frame.c 2010-10-22 23:00:24.000000000 +0800 >>> @@ -81,7 +81,6 @@ struct fc_frame *fc_frame_alloc_fill(str >>> fill = 4 - fill; >>> fp = _fc_frame_alloc(payload_len + fill); >>> if (fp) { >>> - memset((char *) fr_hdr(fp) + payload_len, 0, fill); >>> /* trim is OK, we just allocated it so there are no >>> fragments */ >>> skb_trim(fp_skb(fp), >>> payload_len + sizeof(struct fc_frame_header)); >>> _______________________________________________ >>> devel mailing list >>> [email protected] >>> http://www.open-fcoe.org/mailman/listinfo/devel >> _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
