Hi David,
On Wed, Jun 22, 2016 at 6:04 PM, David Howells <dhowe...@redhat.com> wrote:
> Make rxrpc_send_packet() take a connection not a transport as part of the
> phasing out of the rxrpc_transport struct.
>
> Whilst we're at it, rename the function to rxrpc_send_data_packet() to
> differentiate it from the other packet sending functions.
>
> Signed-off-by: David Howells <dhowe...@redhat.com>
This is now upstream commit 985a5c824a52e9f7
> --- a/net/rxrpc/output.c
> +++ b/net/rxrpc/output.c
> @@ -338,7 +338,7 @@ EXPORT_SYMBOL(rxrpc_kernel_abort_call);
> /*
> * send a packet through the transport endpoint
> */
> -int rxrpc_send_packet(struct rxrpc_transport *trans, struct sk_buff *skb)
> +int rxrpc_send_data_packet(struct rxrpc_connection *conn, struct sk_buff
> *skb)
> {
> struct kvec iov[1];
> struct msghdr msg;
> @@ -349,30 +349,30 @@ int rxrpc_send_packet(struct rxrpc_transport *trans,
> struct sk_buff *skb)
net/rxrpc/output.c: In function ‘rxrpc_send_data_packet’:
net/rxrpc/output.c:252: warning: ‘ret’ may be used uninitialized in
this function
(line number is from current mainline)
> iov[0].iov_base = skb->head;
> iov[0].iov_len = skb->len;
>
> - msg.msg_name = &trans->peer->srx.transport.sin;
> - msg.msg_namelen = sizeof(trans->peer->srx.transport.sin);
> + msg.msg_name = &conn->params.peer->srx.transport;
> + msg.msg_namelen = conn->params.peer->srx.transport_len;
> msg.msg_control = NULL;
> msg.msg_controllen = 0;
> msg.msg_flags = 0;
>
> /* send the packet with the don't fragment bit set if we currently
> * think it's small enough */
> - if (skb->len - sizeof(struct rxrpc_wire_header) <
> trans->peer->maxdata) {
> - down_read(&trans->local->defrag_sem);
> + if (skb->len - sizeof(struct rxrpc_wire_header) <
> conn->params.peer->maxdata) {
> + down_read(&conn->params.local->defrag_sem);
If this branch is not taken...
> /* send the packet by UDP
> * - returns -EMSGSIZE if UDP would have to fragment the
> packet
> * to go out of the interface
> * - in which case, we'll have processed the ICMP error
> * message and update the peer record
> */
> - ret = kernel_sendmsg(trans->local->socket, &msg, iov, 1,
> + ret = kernel_sendmsg(conn->params.local->socket, &msg, iov, 1,
> iov[0].iov_len);
>
> - up_read(&trans->local->defrag_sem);
> + up_read(&conn->params.local->defrag_sem);
> if (ret == -EMSGSIZE)
> goto send_fragmentable;
>
> - _leave(" = %d [%u]", ret, trans->peer->maxdata);
> + _leave(" = %d [%u]", ret, conn->params.peer->maxdata);
> return ret;
> }
>
> @@ -380,21 +380,28 @@ send_fragmentable:
> /* attempt to send this message with fragmentation enabled */
> _debug("send fragment");
>
> - down_write(&trans->local->defrag_sem);
> - opt = IP_PMTUDISC_DONT;
> - ret = kernel_setsockopt(trans->local->socket, SOL_IP, IP_MTU_DISCOVER,
> - (char *) &opt, sizeof(opt));
> - if (ret == 0) {
> - ret = kernel_sendmsg(trans->local->socket, &msg, iov, 1,
> - iov[0].iov_len);
> -
> - opt = IP_PMTUDISC_DO;
> - kernel_setsockopt(trans->local->socket, SOL_IP,
> - IP_MTU_DISCOVER, (char *) &opt,
> sizeof(opt));
> + down_write(&conn->params.local->defrag_sem);
> +
> + switch (conn->params.local->srx.transport.family) {
> + case AF_INET:
> + opt = IP_PMTUDISC_DONT;
> + ret = kernel_setsockopt(conn->params.local->socket,
> + SOL_IP, IP_MTU_DISCOVER,
> + (char *)&opt, sizeof(opt));
> + if (ret == 0) {
> + ret = kernel_sendmsg(conn->params.local->socket,
> &msg, iov, 1,
> + iov[0].iov_len);
> +
> + opt = IP_PMTUDISC_DO;
> + kernel_setsockopt(conn->params.local->socket, SOL_IP,
> + IP_MTU_DISCOVER,
> + (char *)&opt, sizeof(opt));
> + }
> + break;
... and none of the cases (current upstream also has AF_INET6 if
CONFIG_AF_RXRPC_IPV6 is enabled) match ...
> }
>
> - up_write(&trans->local->defrag_sem);
> - _leave(" = %d [frag %u]", ret, trans->peer->maxdata);
> + up_write(&conn->params.local->defrag_sem);
> + _leave(" = %d [frag %u]", ret, conn->params.peer->maxdata);
> return ret;
... then ret is not initialized.
I didn't create a patch, as I'm not sure this is a false positive or not.
Is it possible that none of the cases match?
> }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
