On Sun, Jul 01, 2018 at 08:51:55PM +0200, Julia Lawall wrote: > > > On Sun, 1 Jul 2018, Joe Perches wrote: > > > On Sun, 2018-07-01 at 19:32 +0200, Julia Lawall wrote: > > > Comparing an int to a size, which is unsigned, causes the int to become > > > unsigned, giving the wrong result. > > > > > > The semantic match that finds this problem is as follows: > > > (http://coccinelle.lip6.fr/) > > > > Great, thanks. > > > > But what about the ones in net/smc like: > > > > > net/smc/smc_clc.c: > > > > > > len = kernel_sendmsg(smc->clcsock, &msg, &vec, 1, > > > sizeof(struct smc_clc_msg_decline)); > > > if (len < sizeof(struct smc_clc_msg_decline)) > > > > Are those detected by the semantic match and ignored? > > I wasn't sure how to justify that kernel_sendmsg returns a negative value. > If it is the case, I can send the patch. I only found this in one file, > but there were multiple occurrences. > In theory, Smatch is supposed to know return values but kernel_sendmsg() is too complicated for Smatch. It's a tricky thing... That particular check is correct and deliberate, but there is another check which is wrong. net/smc/smc_clc.c 369 len = kernel_sendmsg(smc->clcsock, &msg, &vec, 1, 370 sizeof(struct smc_clc_msg_decline)); 371 if (len < sizeof(struct smc_clc_msg_decline)) 372 smc->sk.sk_err = EPROTO; 373 if (len < 0) 374 smc->sk.sk_err = -len; If it's invalid we set an error code, if it's already an error we preserve the error code. 375 return sock_error(&smc->sk); [ snip ] 442 /* due to the few bytes needed for clc-handshake this cannot block */ 443 len = kernel_sendmsg(smc->clcsock, &msg, vec, i, plen); 444 if (len < sizeof(pclc)) { 445 if (len >= 0) { ^^^^^^^^ This is always true. 446 reason_code = -ENETUNREACH; 447 smc->sk.sk_err = -reason_code; 448 } else { 449 smc->sk.sk_err = smc->clcsock->sk->sk_err; 450 reason_code = -smc->sk.sk_err; 451 } 452 } The other two checks are not type promoted so they also work as intended. This is an interesting sort of bug I've written a Smatch script inspired by your work here. One for the type promotion and one for the impossible condition. I'll let you know how it goes. regards, dan carpenter
