netdev - Re: [PATCH v3] staging: qlge: fix build breakage with dumping enabled

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200903034918.GA227281@f3>
Date:   Thu, 3 Sep 2020 12:49:18 +0900
From:   Benjamin Poirier <benjamin.poirier@...il.com>
To:     Coiby Xu <coiby.xu@...il.com>
Cc:     devel@...verdev.osuosl.org, Manish Chopra <manishc@...vell.com>,
        "supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER" 
        <GR-Linux-NIC-Dev@...vell.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        "open list:QLOGIC QLGE 10Gb ETHERNET DRIVER" <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] staging: qlge: fix build breakage with dumping enabled

On 2020-09-02 22:00 +0800, Coiby Xu wrote:
> This fixes commit 0107635e15ac
> ("staging: qlge: replace pr_err with netdev_err") which introduced an
> build breakage of missing `struct ql_adapter *qdev` for some functions
> and a warning of type mismatch with dumping enabled, i.e.,
> 
> $ make CFLAGS_MODULE="QL_ALL_DUMP=1 QL_OB_DUMP=1 QL_CB_DUMP=1 \
>   QL_IB_DUMP=1 QL_REG_DUMP=1 QL_DEV_DUMP=1" M=drivers/staging/qlge
> 
> qlge_dbg.c: In function ‘ql_dump_ob_mac_rsp’:
> qlge_dbg.c:2051:13: error: ‘qdev’ undeclared (first use in this function); did you mean ‘cdev’?
>  2051 |  netdev_err(qdev->ndev, "%s\n", __func__);
>       |             ^~~~
> qlge_dbg.c: In function ‘ql_dump_routing_entries’:
> qlge_dbg.c:1435:10: warning: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘int’ [-Wformat=]
>  1435 |        "%s: Routing Mask %d = 0x%.08x\n",
>       |         ~^
>       |          |
>       |          char *
>       |         %d
>  1436 |        i, value);
>       |        ~
>       |        |
>       |        int
> qlge_dbg.c:1435:37: warning: format ‘%x’ expects a matching ‘unsigned int’ argument [-Wformat=]
>  1435 |        "%s: Routing Mask %d = 0x%.08x\n",
>       |                                 ~~~~^
>       |                                     |
>       |                                     unsigned int
> 
> Fixes: 0107635e15ac ("staging: qlge: replace pr_err with netdev_err")
> Reported-by: Benjamin Poirier <benjamin.poirier@...il.com>
> Suggested-by: Benjamin Poirier <benjamin.poirier@...il.com>
> Signed-off-by: Coiby Xu <coiby.xu@...il.com>
> ---

Thanks for following up on this issue.

[...]
> @@ -1632,8 +1635,8 @@ void ql_dump_wqicb(struct wqicb *wqicb)
> 
>  void ql_dump_tx_ring(struct tx_ring *tx_ring)
>  {
> -	if (!tx_ring)
> -		return;
> +	struct ql_adapter *qdev = tx_ring->qdev;
> +
>  	netdev_err(qdev->ndev, "===================== Dumping tx_ring %d ===============\n",
>  		   tx_ring->wq_id);
>  	netdev_err(qdev->ndev, "tx_ring->base = %p\n", tx_ring->wq_base);

Did you actually check to confirm that the test can be removed?

This is something that you should mention in the changelog at the very
least since that change is not directly about fixing the build breakage
and if it's wrong, it can lead to null pointer deref.