linux-kernel - Re: [PATCH] test_firmware: fix end of loop test in upload_read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 11 May 2022 14:41:40 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Luis Chamberlain <mcgrof@...nel.org>,
        Jakob Koschel <jakobkoschel@...il.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Russ Weight <russell.h.weight@...el.com>,
        Tianfei zhang <tianfei.zhang@...el.com>,
        Lee Jones <lee.jones@...aro.org>,
        Shawn Guo <shawn.guo@...aro.org>, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] test_firmware: fix end of loop test in upload_read_show()

On Mon, May 09, 2022 at 03:36:53PM -0700, Luis Chamberlain wrote:
> > This test is reading out of bounds.  Another fix would be to write it
> > as:
> > 
> > 	if (list_entry_is_head(tst, &test_upload_list, node)) {
> > 
> > But there is a desire to make it impossible to access the list iterator
> > outside the loop.  Linus was drafting alternative list macros but I
> > don't know the status of that.
> 
> Fine to get these fixes merged, but it would seem test firmware
> would be low on the list of places to fix. Either way I'm happy
> for this to go in.
> 

I'm just fixing new static checker bugs as they are added, but you
made me curiuos to see if there were old known bugs still.

The following functions are still buggy:
wd719x_interrupt()
nvkm_clk_ustate_update()

Most of the others are Smatch false positives, but some are a bit tricky
and I'm not sure either way.  I marked them as false positives if the
author seemed confident that no end of loop test was required.

Bugs:
drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c:489 nvkm_clk_ustate_update() warn: iterator used outside loop: 'pstate'
drivers/scsi/wd719x.c:691 wd719x_interrupt() warn: iterator used outside loop: 'scb'
lib/test_firmware.c:1408 upload_read_show() warn: iterator used outside loop: 'tst'

Tricky:
drivers/net/ethernet/mellanox/mlx4/alloc.c:379 __mlx4_alloc_from_zone() warn: iterator used outside loop: 'curr_node'

False positives:
drivers/usb/host/uhci-q.c:466 link_async() warn: iterator used outside loop: 'pqh'
drivers/infiniband/core/mad.c:968 ib_get_rmpp_segment() warn: iterator used outside loop: 'mad_send_wr->cur_seg'
drivers/infiniband/hw/hfi1/tid_rdma.c:1280 kern_alloc_tids() warn: iterator used outside loop: 'group'
drivers/staging/rtl8192e/rtl819x_TSProc.c:260 SearchAdmitTRStream() warn: iterator used outside loop: 'pRet'
drivers/perf/xgene_pmu.c:1487 acpi_get_pmu_hw_inf() warn: iterator used outside loop: 'rentry'
drivers/perf/thunderx2_pmu.c:814 tx2_uncore_pmu_init_dev() warn: iterator used outside loop: 'rentry'
drivers/gpu/drm/vc4/vc4_dsi.c:769 vc4_dsi_encoder_disable() warn: iterator used outside loop: 'iter'
drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c:111 nvkm_control_mthd_pstate_attr() warn: iterator used outside loop: 'pstate'
drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c:283 nvkm_pstate_prog() warn: iterator used outside loop: 'pstate'
drivers/gpu/drm/panfrost/panfrost_mmu.c:203 panfrost_mmu_as_get() warn: iterator used outside loop: 'lru_mmu'
drivers/net/wireless/ath/ath6kl/htc_mbox.c:107 ath6kl_credit_init() warn: iterator used outside loop: 'cur_ep_dist'
net/xfrm/xfrm_ipcomp.c:246 ipcomp_free_tfms() warn: iterator used outside loop: 'pos'

regards,
dan carpenter