On 17.02.22 11:27, Bjørn Mork wrote: > Oliver Neukum <oneukum@xxxxxxxx> writes: > >> Hi, >> >> going through the USB network drivers looking for ways >> a malicious device could do us harm I found drivers taking >> the alignment coming from the device for granted. >> >> An example can be seen in qmi_wwan: >> >> while (offset + qmimux_hdr_sz < skb->len) { >> hdr = (struct qmimux_hdr*)(skb->data + offset); >> len = be16_to_cpu(hdr->pkt_len); >> >> As you can see the driver accesses stuff coming from the device with the >> expectation >> that it keep to natural alignment. On some architectures that is a way a >> device could use to do bad things to a host. What is to be done about >> that? > We can deal with this the same way we deal with hostile hot-plugged CPUs > or memory modules. Yes. That is a basic decision that needs to be made > Yes, the aligment should probably be verified. But there are so many > ways a hostile network adapter can mess with us than I don't buy the > "malicious device" argument... Sure, so what is the level of damage that is acceptable? > > FWIW, the more recent rmnet demuxing implementation from Qualcomm seems > to suffer from the same problem. > > > struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb, > struct rmnet_port *port) > { > struct rmnet_map_header *maph; > struct sk_buff *skbn; > u32 packet_len; > > if (skb->len == 0) > return NULL; > > maph = (struct rmnet_map_header *)skb->data; > packet_len = ntohs(maph->pkt_len) + sizeof(struct rmnet_map_header); > > > (this implementation moves skb->data by packet_len instead of doing the > offset calculation, but I don't think that makes any difference?) > > I guess there is no alignment guarantee here, whether the device is > malicious or not. So we probably have to deal with unaligned accesses to > maph/hdr->pkt_len? Yes, as far as I can tell a device is fully in spec if it sends frames as tightly packed as possible, so this is simply a bug, not a security issue. Regards Oliver