Message ID | 20230706111700.14305-1-dzm91@hust.edu.cn |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9f45:0:b0:3ea:f831:8777 with SMTP id v5csp2512109vqx; Thu, 6 Jul 2023 05:11:37 -0700 (PDT) X-Google-Smtp-Source: APBJJlFhX+9bLoYSf6raHXCXBTRQSVNhJnWUMCCT/pk5RIjZx7j8EzAaQGlcEq2zW10fjXbv08nx X-Received: by 2002:a17:902:d4ce:b0:1b8:7e53:70f with SMTP id o14-20020a170902d4ce00b001b87e53070fmr2275041plg.27.1688645497046; Thu, 06 Jul 2023 05:11:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1688645497; cv=none; d=google.com; s=arc-20160816; b=pQvyxQ3tvYkaul5jkxuQelHAQ0yp31habbAo3fk/Zqkf0vkm5lDi3P76TQVWB/57s9 QH3y5xpekDD4NEzd8yekCbLWDD+JBkG7nLaC6DHA+zmIaqnWHyipbuZmSoeCRBWEomJj GROU95O2OrrWL7p6Dc6RAmPALMIv3Va+AC+bsROlKC2aSB7GF+3NncK9ttUxqje+gkno 3R3kB21XWK9K+NzKrxamP9KxRNj1BHAxSkMATp7CXSbcfsRXhkzaXqS8bYcc9DbPb7Nr FIZ7aBxPbDQu3NPLkfXZHUICNGpnCn9PHnpBc0QxeRiZ+gmTdwq4ixFUhWADKq/4L/7b v9CQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=xxZlC0G3QtZ7+XhZAwA5z4tLTrQUwBftO4S+9f+68BU=; fh=gO693ysgJfRulKfbmw7tTxOng7NqOQZxUW+7GzpAQg8=; b=zLaI/p2w2Hp3Aa6MXFL2EX+82MM8ja6S0lGP7aDAT3lisgE1qTMStCgZ69I73nfUiT 5j59GVpQ8lBxvSNUhPTb4s2mzBB605buARGPry1YWpOZvEbloMDdFBs0lDWj3/tOuhkd BO/rmuYtZktOqnxNMpx3aJGBbFt0m8PVBCQ31gvsFEYGSwDdaWbcdexO9MzD9QjDkOfn GPNIpG6/qtSoDxMN+nu+3w3ExqpNvt4uW9h2Gq/PTBQTfSzlTZYtLldHqpVCvvz79m4s fm4GQM3+96XUyjXbnAv09tTp5hKfzD8WKjFEoAMRd9M5tW0yaLUrZid1k6itgJmcXRAu Hbag== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id kz4-20020a170902f9c400b001b5219acbbfsi1182176plb.359.2023.07.06.05.11.24; Thu, 06 Jul 2023 05:11:37 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231557AbjGFLRg (ORCPT <rfc822;hadasmailinglist@gmail.com> + 99 others); Thu, 6 Jul 2023 07:17:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59154 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229486AbjGFLRe (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 6 Jul 2023 07:17:34 -0400 Received: from hust.edu.cn (unknown [202.114.0.240]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 70E47170F; Thu, 6 Jul 2023 04:17:33 -0700 (PDT) Received: from pride-poweredge-r740.. ([172.16.0.254]) (user=dzm91@hust.edu.cn mech=LOGIN bits=0) by mx1.hust.edu.cn with ESMTP id 366BH8YK013936-366BH8YN013936 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 6 Jul 2023 19:17:13 +0800 From: Dongliang Mu <dzm91@hust.edu.cn> To: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= <toke@toke.dk>, Kalle Valo <kvalo@kernel.org> Cc: Dongliang Mu <dzm91@hust.edu.cn>, linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v3] wifi: ath9k: fix printk specifier Date: Thu, 6 Jul 2023 19:17:00 +0800 Message-Id: <20230706111700.14305-1-dzm91@hust.edu.cn> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-FEAS-AUTH-USER: dzm91@hust.edu.cn X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1770672340918668643?= X-GMAIL-MSGID: =?utf-8?q?1770673140504105559?= |
Series |
[v3] wifi: ath9k: fix printk specifier
|
|
Commit Message
Dongliang Mu
July 6, 2023, 11:17 a.m. UTC
Smatch reports:
ath_pci_probe() warn: argument 4 to %lx specifier is cast from pointer
ath_ahb_probe() warn: argument 4 to %lx specifier is cast from pointer
Fix it by modifying %lx to %p in printk.
Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
---
v2->v3: modify %px to %p in the patch
v1->v2: modify %px to %p in the commit message
drivers/net/wireless/ath/ath9k/ahb.c | 4 ++--
drivers/net/wireless/ath/ath9k/pci.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
Comments
On Thu, 6 Jul 2023 at 13:39, Toke Høiland-Jørgensen <toke@toke.dk> wrote: > > Dongliang Mu <dzm91@hust.edu.cn> writes: > > > Smatch reports: > > > > ath_pci_probe() warn: argument 4 to %lx specifier is cast from pointer > > ath_ahb_probe() warn: argument 4 to %lx specifier is cast from pointer > > > > Fix it by modifying %lx to %p in printk. > > > > Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn> > > Third time is the charm! :) > > Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> But do we *want* to replace %lx with %p? Because this now causes a change in behavior, as the kernel will omit the pointer's address, while before it printed the actual memory address. Using %px would have kept the original behavior. So if the change is desired, it should be noted in the commit message. best Regards, Jonas
On 7/7/23 00:04, Jonas Gorski wrote: > On Thu, 6 Jul 2023 at 13:39, Toke Høiland-Jørgensen <toke@toke.dk> wrote: >> Dongliang Mu <dzm91@hust.edu.cn> writes: >> >>> Smatch reports: >>> >>> ath_pci_probe() warn: argument 4 to %lx specifier is cast from pointer >>> ath_ahb_probe() warn: argument 4 to %lx specifier is cast from pointer >>> >>> Fix it by modifying %lx to %p in printk. >>> >>> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn> >> Third time is the charm! :) >> >> Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> > But do we *want* to replace %lx with %p? Because this now causes a > change in behavior, as the kernel will omit the pointer's address, > while before it printed the actual memory address. Using %px would > have kept the original behavior. This is why I made mistake in the patch submission. I think %px is more suitable and generate a patch, but then checkpatch.pl thinks I should use %p instead. I change the code change, but forget the commit message. > > So if the change is desired, it should be noted in the commit message. > > best Regards, > Jonas
Jonas Gorski <jonas.gorski@gmail.com> writes: > On Thu, 6 Jul 2023 at 13:39, Toke Høiland-Jørgensen <toke@toke.dk> wrote: >> >> Dongliang Mu <dzm91@hust.edu.cn> writes: >> >> > Smatch reports: >> > >> > ath_pci_probe() warn: argument 4 to %lx specifier is cast from pointer >> > ath_ahb_probe() warn: argument 4 to %lx specifier is cast from pointer >> > >> > Fix it by modifying %lx to %p in printk. >> > >> > Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn> >> >> Third time is the charm! :) >> >> Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> > > But do we *want* to replace %lx with %p? Because this now causes a > change in behavior, as the kernel will omit the pointer's address, > while before it printed the actual memory address. Using %px would > have kept the original behavior. So this is basically an informational log message, which really shouldn't be leaking kernel pointer addresses. So I think %p is the right choice here, and if someone wants to see the real address for debugging they should be booting with no_hash_pointers. > So if the change is desired, it should be noted in the commit message. That is a fair point, documenting this in the commit message would be good... -Toke
On 2023/7/7 21:44, Toke Høiland-Jørgensen wrote: > Jonas Gorski <jonas.gorski@gmail.com> writes: > >> On Thu, 6 Jul 2023 at 13:39, Toke Høiland-Jørgensen <toke@toke.dk> wrote: >>> Dongliang Mu <dzm91@hust.edu.cn> writes: >>> >>>> Smatch reports: >>>> >>>> ath_pci_probe() warn: argument 4 to %lx specifier is cast from pointer >>>> ath_ahb_probe() warn: argument 4 to %lx specifier is cast from pointer >>>> >>>> Fix it by modifying %lx to %p in printk. >>>> >>>> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn> >>> Third time is the charm! :) >>> >>> Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> >> But do we *want* to replace %lx with %p? Because this now causes a >> change in behavior, as the kernel will omit the pointer's address, >> while before it printed the actual memory address. Using %px would >> have kept the original behavior. > So this is basically an informational log message, which really > shouldn't be leaking kernel pointer addresses. So I think %p is the > right choice here, and if someone wants to see the real address for > debugging they should be booting with no_hash_pointers. > >> So if the change is desired, it should be noted in the commit message. > That is a fair point, documenting this in the commit message would be > good... Hi Toke, What's the next? Patch v4 with a note at the end, like the following, "Note that %p would by default print a hashed value without leaking the kernel pointer to user space." > -Toke
Dongliang Mu <dzm91@hust.edu.cn> writes: > On 2023/7/7 21:44, Toke Høiland-Jørgensen wrote: >> Jonas Gorski <jonas.gorski@gmail.com> writes: >> >>> On Thu, 6 Jul 2023 at 13:39, Toke Høiland-Jørgensen <toke@toke.dk> wrote: >>>> Dongliang Mu <dzm91@hust.edu.cn> writes: >>>> >>>>> Smatch reports: >>>>> >>>>> ath_pci_probe() warn: argument 4 to %lx specifier is cast from pointer >>>>> ath_ahb_probe() warn: argument 4 to %lx specifier is cast from pointer >>>>> >>>>> Fix it by modifying %lx to %p in printk. >>>>> >>>>> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn> >>>> Third time is the charm! :) >>>> >>>> Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> >>> But do we *want* to replace %lx with %p? Because this now causes a >>> change in behavior, as the kernel will omit the pointer's address, >>> while before it printed the actual memory address. Using %px would >>> have kept the original behavior. >> So this is basically an informational log message, which really >> shouldn't be leaking kernel pointer addresses. So I think %p is the >> right choice here, and if someone wants to see the real address for >> debugging they should be booting with no_hash_pointers. >> >>> So if the change is desired, it should be noted in the commit message. >> That is a fair point, documenting this in the commit message would be >> good... > > Hi Toke, > > What's the next? Patch v4 with a note at the end, like the following, > > "Note that %p would by default print a hashed value without leaking the > kernel pointer to user space." Yes, please send a new version with the explanation added. However, this should include not just *what* is changed, but *why* as well. So maybe something like: "Note that with this change, the pointer address will be printed as a hashed value by default. This is appropriate because the kernel shouldn't leak kernel pointers to user space in an informational message. If someone wants to see the real address for debugging purposes, this can be achieved using the no_hash_pointers kernel option" -Toke
diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c index 9cd12b20b18d..a683757eecd4 100644 --- a/drivers/net/wireless/ath/ath9k/ahb.c +++ b/drivers/net/wireless/ath/ath9k/ahb.c @@ -132,8 +132,8 @@ static int ath_ahb_probe(struct platform_device *pdev) ah = sc->sc_ah; ath9k_hw_name(ah, hw_name, sizeof(hw_name)); - wiphy_info(hw->wiphy, "%s mem=0x%lx, irq=%d\n", - hw_name, (unsigned long)mem, irq); + wiphy_info(hw->wiphy, "%s mem=0x%p, irq=%d\n", + hw_name, mem, irq); return 0; diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c index a09f9d223f3d..12c00c7c1408 100644 --- a/drivers/net/wireless/ath/ath9k/pci.c +++ b/drivers/net/wireless/ath/ath9k/pci.c @@ -988,8 +988,8 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) sc->sc_ah->msi_reg = 0; ath9k_hw_name(sc->sc_ah, hw_name, sizeof(hw_name)); - wiphy_info(hw->wiphy, "%s mem=0x%lx, irq=%d\n", - hw_name, (unsigned long)sc->mem, pdev->irq); + wiphy_info(hw->wiphy, "%s mem=0x%p, irq=%d\n", + hw_name, sc->mem, pdev->irq); return 0;