On Friday 13 September 2013 18:08:10 Simon Wunderlich wrote:
+/**
- batadv_socket_receive_packet - schedule an icmp packet to be sent to
userspace + * on an icmp socket.
- @socket_client: the socket this packet belongs to
- @icmph: pointer to the header of the icmp packet
- @icmp_len: total length of the icmp packet
- */
static void batadv_socket_add_packet(struct batadv_socket_client *socket_client, - struct batadv_icmp_packet_rr
*icmp_packet,
struct batadv_icmp_header *icmph, size_t icmp_len){ struct batadv_socket_packet *socket_packet;
size_t len;
socket_packet = kmalloc(sizeof(*socket_packet), GFP_ATOMIC);
if (!socket_packet) return;
len = icmp_len;
/* check the maximum length before filling the buffer */
if (len > sizeof(socket_packet->icmp_packet))
len = sizeof(socket_packet->icmp_packet);INIT_LIST_HEAD(&socket_packet->list);
- memcpy(&socket_packet->icmp_packet, icmp_packet, icmp_len);
- memcpy(&socket_packet->icmp_packet, icmph, icmp_len);
Shouldn't "len" be used here ?
Besides, if we make everything generic batadv_socket_packet->icmp_packet should not be hard-coded to batadv_icmp_packet_rr but the largest available ICMP packet type ?
+/**
- batadv_recv_my_icmp_packet - receive an icmp packet locally
- @bat_priv: the bat priv with all the soft interface information
- @skb: icmp packet to process
- Returns NET_RX_SUCCESS if the packet has been consumed or NET_RX_DROP
- otherwise.
- */
static int batadv_recv_my_icmp_packet(struct batadv_priv *bat_priv,
struct sk_buff *skb, size_t icmp_len)
struct sk_buff *skb){ struct batadv_hard_iface *primary_if = NULL; struct batadv_orig_node *orig_node = NULL;
- struct batadv_icmp_packet_rr *icmp_packet;
- struct batadv_icmp_header *icmph; int ret = NET_RX_DROP;
- icmp_packet = (struct batadv_icmp_packet_rr *)skb->data;
icmph = (struct batadv_icmp_header *)skb->data;
/* add data to device queue */
- if (icmp_packet->icmph.msg_type != BATADV_ECHO_REQUEST) {
batadv_socket_receive_packet(icmp_packet, icmp_len);
- if (icmph->msg_type != BATADV_ECHO_REQUEST) {
if (skb_linearize(skb) < 0)goto out;batadv_socket_receive_packet(icmph, skb->len);
Wouldn't it be better to dump unkown icmp types for us instead of copying everything to user space ?
Same is true for batadv_socket_write(). We should use the icmp header and not assume icmp echo.
Cheers, Marek