Re: check idea: warn when mixing signedness in ?: operator (got bitten by this recently)

[Dan Carpenter <dan.carpenter@oracle.com>]

On Tue, Apr 20, 2021 at 02:44:08PM +0200, Aurélien Aptel wrote:
> Hi Dan,
> 
> Dan Carpenter <dan.carpenter@oracle.com> writes:
> > Thanks for the idea.  I can implement something like that in Smatch.
> > I'll run the attached check over the kernel and see what it turns up.
> 
> I've only used sparse I think (make C=1) I need to lookup how to use Smatch.
> 
> > It says that it's only checking assignments but the trick is that
> > Smatch creates fake assignments in the background for passing parameters
> > or returning.  So "return a ? uint_val : -ENOMEM;" will trigger an error
> > message.
> 
> Sounds good.
> 
> > If there are too many false positives when I test this tonight, then I
> > may make is_suspicious_int() more strict.
> 
> If that's any help, the exact bug where we hit this is currently in
> fs/cifs/file.c in collect_uncached_write_data(), this line:
> 
> 	ctx->rc = (rc == 0) ? ctx->total_len : rc;
> 
> Hopefully it shows up in your tests.        
>

Yeah.  It finds it.  :)  It works pretty well.  The temptation is to
ignore left shifts.  Otherwise I think I will just push this.

regards,
dan carpenter

fs/f2fs/segment.c:847 __remove_dirty_segment() warn: check sign expansion for '-1'
fs/cifs/file.c:3177 collect_uncached_write_data() warn: check sign expansion for 'rc'
drivers/staging/rtl8188eu/core/rtw_xmit.c:1006 rtw_xmitframe_coalesce() warn: check sign expansion for 'mpdu_len'
drivers/usb/gadget/legacy/inode.c:501 ep_aio_complete() warn: check sign expansion for 'req->status'
drivers/gpu/drm/nouveau/nouveau_hwmon.c:507 nouveau_in_read() warn: check sign expansion for '-19'
drivers/gpu/drm/nouveau/nouveau_hwmon.c:510 nouveau_in_read() warn: check sign expansion for '-19'
drivers/firmware/arm_scpi.c:556 scpi_clk_get_val() warn: check sign expansion for 'ret'
drivers/clk/sunxi-ng/ccu_nm.c:158 ccu_nm_round_rate() warn: check sign expansion for '1 << nm->m.width'
drivers/clk/sunxi-ng/ccu_nm.c:202 ccu_nm_set_rate() warn: check sign expansion for '1 << nm->m.width'
drivers/clk/sunxi-ng/ccu_nkmp.c:149 ccu_nkmp_round_rate() warn: check sign expansion for '1 << nkmp->m.width'
drivers/clk/sunxi-ng/ccu_nkmp.c:151 ccu_nkmp_round_rate() warn: check sign expansion for '1 << ((1 << nkmp->p.width) - 1)'
drivers/clk/sunxi-ng/ccu_nkmp.c:180 ccu_nkmp_set_rate() warn: check sign expansion for '1 << nkmp->m.width'
drivers/clk/sunxi-ng/ccu_nkmp.c:182 ccu_nkmp_set_rate() warn: check sign expansion for '1 << ((1 << nkmp->p.width) - 1)'
drivers/clk/sunxi-ng/ccu_nkm.c:120 ccu_nkm_round_rate() warn: check sign expansion for '1 << nkm->m.width'
drivers/clk/sunxi-ng/ccu_nkm.c:160 ccu_nkm_set_rate() warn: check sign expansion for '1 << nkm->m.width'
drivers/net/ethernet/broadcom/bnxt/bnxt.c:9785 bnxt_show_temp() warn: check sign expansion for 'rc'
drivers/soc/aspeed/aspeed-lpc-snoop.c:98 snoop_file_read() warn: check sign expansion for 'ret'
samples/kfifo/bytestream-example.c:126 fifo_write() warn: check sign expansion for 'ret'
samples/kfifo/bytestream-example.c:142 fifo_read() warn: check sign expansion for 'ret'
samples/kfifo/record-example.c:133 fifo_write() warn: check sign expansion for 'ret'
samples/kfifo/record-example.c:149 fifo_read() warn: check sign expansion for 'ret'
samples/kfifo/inttype-example.c:119 fifo_write() warn: check sign expansion for 'ret'
samples/kfifo/inttype-example.c:135 fifo_read() warn: check sign expansion for 'ret'
net/sunrpc/svcsock.c:1177 svc_tcp_sendto() warn: check sign expansion for 'err'