| Message ID | 20190528142424.19626-3-geert@linux-m68k.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Assorted fixes discovered with gcc 4.1 | expand |
Geert Uytterhoeven <geert@linux-m68k.org> wrote: > While this is not a real false-positive, I believe it cannot cause harm > in practice, as AF_RXRPC cannot be used with other transport families > than IPv4 and IPv6. Agreed. > --- > net/rxrpc/output.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c > index 004c762c2e8d063c..1473d774d67100c5 100644 > --- a/net/rxrpc/output.c > +++ b/net/rxrpc/output.c > @@ -403,8 +403,10 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb, > > /* send the packet with the don't fragment bit set if we currently > * think it's small enough */ > - if (iov[1].iov_len >= call->peer->maxdata) > + if (iov[1].iov_len >= call->peer->maxdata) { > + ret = 0; > goto send_fragmentable; > + } > > down_read(&conn->params.local->defrag_sem); > Simply setting 0 is wrong. That would give the impression that the thing worked if support for a new transport address family was added and came through this function without full modification (say AF_INET7 becomes a thing). A better way to do things would be to add a default case into the send_fragmentable switch statement that either BUG's or sets -EAFNOSUPPORT. David
On Tue, May 28, 2019 at 4:24 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > With gcc 4.1: > > net/rxrpc/output.c: In function ‘rxrpc_send_data_packet’: > net/rxrpc/output.c:338: warning: ‘ret’ may be used uninitialized in this function > > Indeed, if the first jump to the send_fragmentable label is made, and > the address family is not handled in the switch() statement, ret will be > used uninitialized. > > Fix this by initializing err to zero before the jump, like is already > done for the jump to the done label. > > Fixes: 5a924b8951f835b5 ("rxrpc: Don't store the rxrpc header in the Tx queue sk_buffs") > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > While this is not a real false-positive, I believe it cannot cause harm > in practice, as AF_RXRPC cannot be used with other transport families > than IPv4 and IPv6. This looks like a variant of the infamous bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501 What I don't understand is why clang fails to warn about it with -Wsometimes-uninitialized. (cc clang-built-linux mailing list). Arnd > net/rxrpc/output.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c > index 004c762c2e8d063c..1473d774d67100c5 100644 > --- a/net/rxrpc/output.c > +++ b/net/rxrpc/output.c > @@ -403,8 +403,10 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb, > > /* send the packet with the don't fragment bit set if we currently > * think it's small enough */ > - if (iov[1].iov_len >= call->peer->maxdata) > + if (iov[1].iov_len >= call->peer->maxdata) { > + ret = 0; > goto send_fragmentable; > + } > > down_read(&conn->params.local->defrag_sem); >
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c index 004c762c2e8d063c..1473d774d67100c5 100644 --- a/net/rxrpc/output.c +++ b/net/rxrpc/output.c @@ -403,8 +403,10 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb, /* send the packet with the don't fragment bit set if we currently * think it's small enough */ - if (iov[1].iov_len >= call->peer->maxdata) + if (iov[1].iov_len >= call->peer->maxdata) { + ret = 0; goto send_fragmentable; + } down_read(&conn->params.local->defrag_sem);
With gcc 4.1: net/rxrpc/output.c: In function ‘rxrpc_send_data_packet’: net/rxrpc/output.c:338: warning: ‘ret’ may be used uninitialized in this function Indeed, if the first jump to the send_fragmentable label is made, and the address family is not handled in the switch() statement, ret will be used uninitialized. Fix this by initializing err to zero before the jump, like is already done for the jump to the done label. Fixes: 5a924b8951f835b5 ("rxrpc: Don't store the rxrpc header in the Tx queue sk_buffs") Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> --- While this is not a real false-positive, I believe it cannot cause harm in practice, as AF_RXRPC cannot be used with other transport families than IPv4 and IPv6. --- net/rxrpc/output.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)