Re: [PATCH] bus: mhi: core: debugfs: Use correct format specifiers for addresses
On Sat, Sep 26, 2020 at 02:46:26PM +0200, Greg KH wrote: > On Sat, Sep 26, 2020 at 12:46:04PM +0530, Manivannan Sadhasivam wrote: > > On Sat, Sep 26, 2020 at 07:39:14AM +0200, Greg KH wrote: > > > On Sat, Sep 26, 2020 at 10:57:42AM +0530, Manivannan Sadhasivam wrote: > > > > On Fri, Sep 25, 2020 at 12:01:54PM -0600, Jeffrey Hugo wrote: > > > > > On 9/25/2020 11:16 AM, Manivannan Sadhasivam wrote: > > > > > > For exposing the addresses of read/write pointers and doorbell > > > > > > register, > > > > > > let's use the correct format specifiers. This fixes the following > > > > > > issues > > > > > > generated using W=1 build in ARM32 and reported by Kbuild bot: > > > > > > > > > > > > All warnings (new ones prefixed by >>): > > > > > > > > > > > > > > drivers/bus/mhi/core/debugfs.c:75:7: warning: format specifies > > > > > > > > type 'unsigned long long' but the argument has type > > > > > > > > 'dma_addr_t' (aka 'unsigned int') [-Wformat] > > > > > >mhi_event->db_cfg.db_val); > > > > > >^~~~ > > > > > > drivers/bus/mhi/core/debugfs.c:123:7: warning: format specifies > > > > > > type 'unsigned long long' but the argument has type 'dma_addr_t' > > > > > > (aka 'unsigned int') [-Wformat] > > > > > >mhi_chan->db_cfg.db_val); > > > > > >^~~ > > > > > > 2 warnings generated. > > > > > > > > > > > > drivers/bus/mhi/core/debugfs.c: In function > > > > > > ‘mhi_debugfs_events_show’: > > > > > > drivers/bus/mhi/core/debugfs.c:74:51: warning: cast from pointer to > > > > > > integer of different size [-Wpointer-to-int-cast] > > > > > > seq_printf(m, " local rp: 0x%llx db: 0x%pad\n", (u64)ring->rp, > > > > > > ^ > > > > > > drivers/bus/mhi/core/debugfs.c: In function > > > > > > ‘mhi_debugfs_channels_show’: > > > > > > drivers/bus/mhi/core/debugfs.c:122:7: warning: cast from pointer to > > > > > > integer of different size [-Wpointer-to-int-cast] > > > > > > (u64)ring->rp, (u64)ring->wp, > > > > > > ^ > > > > > > drivers/bus/mhi/core/debugfs.c:122:22: warning: cast from pointer > > > > > > to integer of different size [-Wpointer-to-int-cast] > > > > > > (u64)ring->rp, (u64)ring->wp, > > > > > >^ > > > > > > drivers/bus/mhi/core/debugfs.c:121:62: warning: format ‘%llx’ > > > > > > expects argument of type ‘long long unsigned int’, but argument 5 > > > > > > has type ‘dma_addr_t {aka unsigned int}’ [-Wformat=] > > > > > > seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", > > > > > > ~~~^ > > > > > > %x > > > > > > drivers/bus/mhi/core/debugfs.c:123:7: > > > > > > mhi_chan->db_cfg.db_val); > > > > > > > > > > > > Reported-by: kernel test robot > > > > > > Signed-off-by: Manivannan Sadhasivam > > > > > > > > > > > > --- > > > > > > > > > > > > Greg: This fixes the issue seen while testing the > > > > > > char-misc/char-misc-testing > > > > > > branch. > > > > > > > > > > > > drivers/bus/mhi/core/debugfs.c | 10 +- > > > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > > > > > diff --git a/drivers/bus/mhi/core/debugfs.c > > > > > > b/drivers/bus/mhi/core/debugfs.c > > > > > > index 53d05a8e168d..2536ff92b76f 100644 > > > > > > --- a/drivers/bus/mhi/core/debugfs.c > > > > > > +++ b/drivers/bus/mhi/core/debugfs.c > > > > > > @@ -71,8 +71,8 @@ static int mhi_debugfs_events_show(struct > > > > > > seq_file *m, void *d) > > > > > > seq_printf(m, " rp: 0x%llx wp: 0x%llx", er_ctxt->rp, > > > > > >er_ctxt->wp); > > > > > > - seq_printf(m, " local rp: 0x%llx db: 0x%llx\n", > > > > > > (u64)ring->rp, > > > > > > - mhi_event->db_cfg.db_val); > > > > > > + seq_printf(m, " local rp: 0x%px db: 0x%pad\n", ring->rp, > > > > > > + &mhi_event->db_cfg.db_val); > > > > > > } > > > > > > return 0; > > > > > > @@ -118,9 +118,9 @@ static int mhi_debugfs_channels_show(struct > > > > > > seq_file *m, void *d) > > > > > > seq_printf(m, " base: 0x%llx len: 0x%llx wp: 0x%llx", > > > > > >chan_ctxt->rbase, chan_ctxt->rlen, > > > > > > chan_ctxt->wp); > > > > > > - seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: > > > > > > 0x%llx\n", > > > > > > - (u64)ring->rp, (u64)ring->wp, > > > > > > - mhi_chan->db_cfg.db_val); > > > > > > + seq_printf(m, " local rp: 0x%px local wp: 0x%px db: > > > > > > 0x%pad\n", > > > > > > + ring->rp, ring->wp, > > > > > > + &mhi_chan->db_cfg.db_val); > > > > > > } > > > > > > return 0; > >
Re: [PATCH] bus: mhi: core: debugfs: Use correct format specifiers for addresses
On Sat, Sep 26, 2020 at 12:46:04PM +0530, Manivannan Sadhasivam wrote: > On Sat, Sep 26, 2020 at 07:39:14AM +0200, Greg KH wrote: > > On Sat, Sep 26, 2020 at 10:57:42AM +0530, Manivannan Sadhasivam wrote: > > > On Fri, Sep 25, 2020 at 12:01:54PM -0600, Jeffrey Hugo wrote: > > > > On 9/25/2020 11:16 AM, Manivannan Sadhasivam wrote: > > > > > For exposing the addresses of read/write pointers and doorbell > > > > > register, > > > > > let's use the correct format specifiers. This fixes the following > > > > > issues > > > > > generated using W=1 build in ARM32 and reported by Kbuild bot: > > > > > > > > > > All warnings (new ones prefixed by >>): > > > > > > > > > > > > drivers/bus/mhi/core/debugfs.c:75:7: warning: format specifies > > > > > > > type 'unsigned long long' but the argument has type 'dma_addr_t' > > > > > > > (aka 'unsigned int') [-Wformat] > > > > >mhi_event->db_cfg.db_val); > > > > >^~~~ > > > > > drivers/bus/mhi/core/debugfs.c:123:7: warning: format specifies > > > > > type 'unsigned long long' but the argument has type 'dma_addr_t' (aka > > > > > 'unsigned int') [-Wformat] > > > > >mhi_chan->db_cfg.db_val); > > > > >^~~ > > > > > 2 warnings generated. > > > > > > > > > > drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_events_show’: > > > > > drivers/bus/mhi/core/debugfs.c:74:51: warning: cast from pointer to > > > > > integer of different size [-Wpointer-to-int-cast] > > > > > seq_printf(m, " local rp: 0x%llx db: 0x%pad\n", (u64)ring->rp, > > > > > ^ > > > > > drivers/bus/mhi/core/debugfs.c: In function > > > > > ‘mhi_debugfs_channels_show’: > > > > > drivers/bus/mhi/core/debugfs.c:122:7: warning: cast from pointer to > > > > > integer of different size [-Wpointer-to-int-cast] > > > > > (u64)ring->rp, (u64)ring->wp, > > > > > ^ > > > > > drivers/bus/mhi/core/debugfs.c:122:22: warning: cast from pointer to > > > > > integer of different size [-Wpointer-to-int-cast] > > > > > (u64)ring->rp, (u64)ring->wp, > > > > >^ > > > > > drivers/bus/mhi/core/debugfs.c:121:62: warning: format ‘%llx’ expects > > > > > argument of type ‘long long unsigned int’, but argument 5 has type > > > > > ‘dma_addr_t {aka unsigned int}’ [-Wformat=] > > > > > seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", > > > > > ~~~^ > > > > > %x > > > > > drivers/bus/mhi/core/debugfs.c:123:7: > > > > > mhi_chan->db_cfg.db_val); > > > > > > > > > > Reported-by: kernel test robot > > > > > Signed-off-by: Manivannan Sadhasivam > > > > > > > > > > --- > > > > > > > > > > Greg: This fixes the issue seen while testing the > > > > > char-misc/char-misc-testing > > > > > branch. > > > > > > > > > > drivers/bus/mhi/core/debugfs.c | 10 +- > > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/drivers/bus/mhi/core/debugfs.c > > > > > b/drivers/bus/mhi/core/debugfs.c > > > > > index 53d05a8e168d..2536ff92b76f 100644 > > > > > --- a/drivers/bus/mhi/core/debugfs.c > > > > > +++ b/drivers/bus/mhi/core/debugfs.c > > > > > @@ -71,8 +71,8 @@ static int mhi_debugfs_events_show(struct seq_file > > > > > *m, void *d) > > > > > seq_printf(m, " rp: 0x%llx wp: 0x%llx", er_ctxt->rp, > > > > > er_ctxt->wp); > > > > > - seq_printf(m, " local rp: 0x%llx db: 0x%llx\n", > > > > > (u64)ring->rp, > > > > > -mhi_event->db_cfg.db_val); > > > > > + seq_printf(m, " local rp: 0x%px db: 0x%pad\n", ring->rp, > > > > > +&mhi_event->db_cfg.db_val); > > > > > } > > > > > return 0; > > > > > @@ -118,9 +118,9 @@ static int mhi_debugfs_channels_show(struct > > > > > seq_file *m, void *d) > > > > > seq_printf(m, " base: 0x%llx len: 0x%llx wp: 0x%llx", > > > > > chan_ctxt->rbase, chan_ctxt->rlen, > > > > > chan_ctxt->wp); > > > > > - seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: > > > > > 0x%llx\n", > > > > > -(u64)ring->rp, (u64)ring->wp, > > > > > -mhi_chan->db_cfg.db_val); > > > > > + seq_printf(m, " local rp: 0x%px local wp: 0x%px db: > > > > > 0x%pad\n", > > > > > +ring->rp, ring->wp, > > > > > +&mhi_chan->db_cfg.db_val); > > > > > } > > > > > return 0; > > > > > > > > > > > > > Documentation/printk-formats.txt seems to point out that %px is > > > > "insecure" > > > > and thus perhaps not preferred. Are we assuming that debugfs is only > > > > accessible by root, and
Re: [PATCH] bus: mhi: core: debugfs: Use correct format specifiers for addresses
On Sat, Sep 26, 2020 at 07:39:14AM +0200, Greg KH wrote: > On Sat, Sep 26, 2020 at 10:57:42AM +0530, Manivannan Sadhasivam wrote: > > On Fri, Sep 25, 2020 at 12:01:54PM -0600, Jeffrey Hugo wrote: > > > On 9/25/2020 11:16 AM, Manivannan Sadhasivam wrote: > > > > For exposing the addresses of read/write pointers and doorbell register, > > > > let's use the correct format specifiers. This fixes the following issues > > > > generated using W=1 build in ARM32 and reported by Kbuild bot: > > > > > > > > All warnings (new ones prefixed by >>): > > > > > > > > > > drivers/bus/mhi/core/debugfs.c:75:7: warning: format specifies type > > > > > > 'unsigned long long' but the argument has type 'dma_addr_t' (aka > > > > > > 'unsigned int') [-Wformat] > > > >mhi_event->db_cfg.db_val); > > > >^~~~ > > > > drivers/bus/mhi/core/debugfs.c:123:7: warning: format specifies > > > > type 'unsigned long long' but the argument has type 'dma_addr_t' (aka > > > > 'unsigned int') [-Wformat] > > > >mhi_chan->db_cfg.db_val); > > > >^~~ > > > > 2 warnings generated. > > > > > > > > drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_events_show’: > > > > drivers/bus/mhi/core/debugfs.c:74:51: warning: cast from pointer to > > > > integer of different size [-Wpointer-to-int-cast] > > > > seq_printf(m, " local rp: 0x%llx db: 0x%pad\n", (u64)ring->rp, > > > > ^ > > > > drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_channels_show’: > > > > drivers/bus/mhi/core/debugfs.c:122:7: warning: cast from pointer to > > > > integer of different size [-Wpointer-to-int-cast] > > > > (u64)ring->rp, (u64)ring->wp, > > > > ^ > > > > drivers/bus/mhi/core/debugfs.c:122:22: warning: cast from pointer to > > > > integer of different size [-Wpointer-to-int-cast] > > > > (u64)ring->rp, (u64)ring->wp, > > > >^ > > > > drivers/bus/mhi/core/debugfs.c:121:62: warning: format ‘%llx’ expects > > > > argument of type ‘long long unsigned int’, but argument 5 has type > > > > ‘dma_addr_t {aka unsigned int}’ [-Wformat=] > > > > seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", > > > > ~~~^ > > > > %x > > > > drivers/bus/mhi/core/debugfs.c:123:7: > > > > mhi_chan->db_cfg.db_val); > > > > > > > > Reported-by: kernel test robot > > > > Signed-off-by: Manivannan Sadhasivam > > > > --- > > > > > > > > Greg: This fixes the issue seen while testing the > > > > char-misc/char-misc-testing > > > > branch. > > > > > > > > drivers/bus/mhi/core/debugfs.c | 10 +- > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/drivers/bus/mhi/core/debugfs.c > > > > b/drivers/bus/mhi/core/debugfs.c > > > > index 53d05a8e168d..2536ff92b76f 100644 > > > > --- a/drivers/bus/mhi/core/debugfs.c > > > > +++ b/drivers/bus/mhi/core/debugfs.c > > > > @@ -71,8 +71,8 @@ static int mhi_debugfs_events_show(struct seq_file > > > > *m, void *d) > > > > seq_printf(m, " rp: 0x%llx wp: 0x%llx", er_ctxt->rp, > > > >er_ctxt->wp); > > > > - seq_printf(m, " local rp: 0x%llx db: 0x%llx\n", > > > > (u64)ring->rp, > > > > - mhi_event->db_cfg.db_val); > > > > + seq_printf(m, " local rp: 0x%px db: 0x%pad\n", ring->rp, > > > > + &mhi_event->db_cfg.db_val); > > > > } > > > > return 0; > > > > @@ -118,9 +118,9 @@ static int mhi_debugfs_channels_show(struct > > > > seq_file *m, void *d) > > > > seq_printf(m, " base: 0x%llx len: 0x%llx wp: 0x%llx", > > > >chan_ctxt->rbase, chan_ctxt->rlen, > > > > chan_ctxt->wp); > > > > - seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: > > > > 0x%llx\n", > > > > - (u64)ring->rp, (u64)ring->wp, > > > > - mhi_chan->db_cfg.db_val); > > > > + seq_printf(m, " local rp: 0x%px local wp: 0x%px db: > > > > 0x%pad\n", > > > > + ring->rp, ring->wp, > > > > + &mhi_chan->db_cfg.db_val); > > > > } > > > > return 0; > > > > > > > > > > Documentation/printk-formats.txt seems to point out that %px is "insecure" > > > and thus perhaps not preferred. Are we assuming that debugfs is only > > > accessible by root, and thus the %px usage here is effectively the same as > > > %pK? > > > > > > > No, this debugfs entry can be read by non-root users also. > > How, the mount point of debugfs is restricted to root only :) > Sigh... I just went with the file permission of 444 :/ > > But
Re: [PATCH] bus: mhi: core: debugfs: Use correct format specifiers for addresses
On Sat, Sep 26, 2020 at 10:57:42AM +0530, Manivannan Sadhasivam wrote: > On Fri, Sep 25, 2020 at 12:01:54PM -0600, Jeffrey Hugo wrote: > > On 9/25/2020 11:16 AM, Manivannan Sadhasivam wrote: > > > For exposing the addresses of read/write pointers and doorbell register, > > > let's use the correct format specifiers. This fixes the following issues > > > generated using W=1 build in ARM32 and reported by Kbuild bot: > > > > > > All warnings (new ones prefixed by >>): > > > > > > > > drivers/bus/mhi/core/debugfs.c:75:7: warning: format specifies type > > > > > 'unsigned long long' but the argument has type 'dma_addr_t' (aka > > > > > 'unsigned int') [-Wformat] > > >mhi_event->db_cfg.db_val); > > >^~~~ > > > drivers/bus/mhi/core/debugfs.c:123:7: warning: format specifies type > > > 'unsigned long long' but the argument has type 'dma_addr_t' (aka > > > 'unsigned int') [-Wformat] > > >mhi_chan->db_cfg.db_val); > > >^~~ > > > 2 warnings generated. > > > > > > drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_events_show’: > > > drivers/bus/mhi/core/debugfs.c:74:51: warning: cast from pointer to > > > integer of different size [-Wpointer-to-int-cast] > > > seq_printf(m, " local rp: 0x%llx db: 0x%pad\n", (u64)ring->rp, > > > ^ > > > drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_channels_show’: > > > drivers/bus/mhi/core/debugfs.c:122:7: warning: cast from pointer to > > > integer of different size [-Wpointer-to-int-cast] > > > (u64)ring->rp, (u64)ring->wp, > > > ^ > > > drivers/bus/mhi/core/debugfs.c:122:22: warning: cast from pointer to > > > integer of different size [-Wpointer-to-int-cast] > > > (u64)ring->rp, (u64)ring->wp, > > >^ > > > drivers/bus/mhi/core/debugfs.c:121:62: warning: format ‘%llx’ expects > > > argument of type ‘long long unsigned int’, but argument 5 has type > > > ‘dma_addr_t {aka unsigned int}’ [-Wformat=] > > > seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", > > > ~~~^ > > > %x > > > drivers/bus/mhi/core/debugfs.c:123:7: > > > mhi_chan->db_cfg.db_val); > > > > > > Reported-by: kernel test robot > > > Signed-off-by: Manivannan Sadhasivam > > > --- > > > > > > Greg: This fixes the issue seen while testing the > > > char-misc/char-misc-testing > > > branch. > > > > > > drivers/bus/mhi/core/debugfs.c | 10 +- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/bus/mhi/core/debugfs.c > > > b/drivers/bus/mhi/core/debugfs.c > > > index 53d05a8e168d..2536ff92b76f 100644 > > > --- a/drivers/bus/mhi/core/debugfs.c > > > +++ b/drivers/bus/mhi/core/debugfs.c > > > @@ -71,8 +71,8 @@ static int mhi_debugfs_events_show(struct seq_file *m, > > > void *d) > > > seq_printf(m, " rp: 0x%llx wp: 0x%llx", er_ctxt->rp, > > > er_ctxt->wp); > > > - seq_printf(m, " local rp: 0x%llx db: 0x%llx\n", (u64)ring->rp, > > > -mhi_event->db_cfg.db_val); > > > + seq_printf(m, " local rp: 0x%px db: 0x%pad\n", ring->rp, > > > +&mhi_event->db_cfg.db_val); > > > } > > > return 0; > > > @@ -118,9 +118,9 @@ static int mhi_debugfs_channels_show(struct seq_file > > > *m, void *d) > > > seq_printf(m, " base: 0x%llx len: 0x%llx wp: 0x%llx", > > > chan_ctxt->rbase, chan_ctxt->rlen, > > > chan_ctxt->wp); > > > - seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", > > > -(u64)ring->rp, (u64)ring->wp, > > > -mhi_chan->db_cfg.db_val); > > > + seq_printf(m, " local rp: 0x%px local wp: 0x%px db: 0x%pad\n", > > > +ring->rp, ring->wp, > > > +&mhi_chan->db_cfg.db_val); > > > } > > > return 0; > > > > > > > Documentation/printk-formats.txt seems to point out that %px is "insecure" > > and thus perhaps not preferred. Are we assuming that debugfs is only > > accessible by root, and thus the %px usage here is effectively the same as > > %pK? > > > > No, this debugfs entry can be read by non-root users also. How, the mount point of debugfs is restricted to root only :) > But the idea here > is to effectively show the addresses to everyone so I don't think we need to > hide it. The term "insecure" applies to kernel log where exposing the address > doesn't make much sense (except for few obvious reasons). Why does normal users need to see a kernel address? What can they do with this? Why can't we use the "normal" hashed way of showing a kernel
Re: [PATCH] bus: mhi: core: debugfs: Use correct format specifiers for addresses
On Fri, Sep 25, 2020 at 12:01:54PM -0600, Jeffrey Hugo wrote: > On 9/25/2020 11:16 AM, Manivannan Sadhasivam wrote: > > For exposing the addresses of read/write pointers and doorbell register, > > let's use the correct format specifiers. This fixes the following issues > > generated using W=1 build in ARM32 and reported by Kbuild bot: > > > > All warnings (new ones prefixed by >>): > > > > > > drivers/bus/mhi/core/debugfs.c:75:7: warning: format specifies type > > > > 'unsigned long long' but the argument has type 'dma_addr_t' (aka > > > > 'unsigned int') [-Wformat] > >mhi_event->db_cfg.db_val); > >^~~~ > > drivers/bus/mhi/core/debugfs.c:123:7: warning: format specifies type > > 'unsigned long long' but the argument has type 'dma_addr_t' (aka 'unsigned > > int') [-Wformat] > >mhi_chan->db_cfg.db_val); > >^~~ > > 2 warnings generated. > > > > drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_events_show’: > > drivers/bus/mhi/core/debugfs.c:74:51: warning: cast from pointer to integer > > of different size [-Wpointer-to-int-cast] > > seq_printf(m, " local rp: 0x%llx db: 0x%pad\n", (u64)ring->rp, > > ^ > > drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_channels_show’: > > drivers/bus/mhi/core/debugfs.c:122:7: warning: cast from pointer to integer > > of different size [-Wpointer-to-int-cast] > > (u64)ring->rp, (u64)ring->wp, > > ^ > > drivers/bus/mhi/core/debugfs.c:122:22: warning: cast from pointer to > > integer of different size [-Wpointer-to-int-cast] > > (u64)ring->rp, (u64)ring->wp, > >^ > > drivers/bus/mhi/core/debugfs.c:121:62: warning: format ‘%llx’ expects > > argument of type ‘long long unsigned int’, but argument 5 has type > > ‘dma_addr_t {aka unsigned int}’ [-Wformat=] > > seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", > > ~~~^ > > %x > > drivers/bus/mhi/core/debugfs.c:123:7: > > mhi_chan->db_cfg.db_val); > > > > Reported-by: kernel test robot > > Signed-off-by: Manivannan Sadhasivam > > --- > > > > Greg: This fixes the issue seen while testing the > > char-misc/char-misc-testing > > branch. > > > > drivers/bus/mhi/core/debugfs.c | 10 +- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/bus/mhi/core/debugfs.c b/drivers/bus/mhi/core/debugfs.c > > index 53d05a8e168d..2536ff92b76f 100644 > > --- a/drivers/bus/mhi/core/debugfs.c > > +++ b/drivers/bus/mhi/core/debugfs.c > > @@ -71,8 +71,8 @@ static int mhi_debugfs_events_show(struct seq_file *m, > > void *d) > > seq_printf(m, " rp: 0x%llx wp: 0x%llx", er_ctxt->rp, > >er_ctxt->wp); > > - seq_printf(m, " local rp: 0x%llx db: 0x%llx\n", (u64)ring->rp, > > - mhi_event->db_cfg.db_val); > > + seq_printf(m, " local rp: 0x%px db: 0x%pad\n", ring->rp, > > + &mhi_event->db_cfg.db_val); > > } > > return 0; > > @@ -118,9 +118,9 @@ static int mhi_debugfs_channels_show(struct seq_file > > *m, void *d) > > seq_printf(m, " base: 0x%llx len: 0x%llx wp: 0x%llx", > >chan_ctxt->rbase, chan_ctxt->rlen, chan_ctxt->wp); > > - seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", > > - (u64)ring->rp, (u64)ring->wp, > > - mhi_chan->db_cfg.db_val); > > + seq_printf(m, " local rp: 0x%px local wp: 0x%px db: 0x%pad\n", > > + ring->rp, ring->wp, > > + &mhi_chan->db_cfg.db_val); > > } > > return 0; > > > > Documentation/printk-formats.txt seems to point out that %px is "insecure" > and thus perhaps not preferred. Are we assuming that debugfs is only > accessible by root, and thus the %px usage here is effectively the same as > %pK? > No, this debugfs entry can be read by non-root users also. But the idea here is to effectively show the addresses to everyone so I don't think we need to hide it. The term "insecure" applies to kernel log where exposing the address doesn't make much sense (except for few obvious reasons). Thanks, Mani > -- > Jeffrey Hugo > Qualcomm Technologies, Inc. is a member of the > Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH] bus: mhi: core: debugfs: Use correct format specifiers for addresses
On 9/25/2020 11:16 AM, Manivannan Sadhasivam wrote: For exposing the addresses of read/write pointers and doorbell register, let's use the correct format specifiers. This fixes the following issues generated using W=1 build in ARM32 and reported by Kbuild bot: All warnings (new ones prefixed by >>): drivers/bus/mhi/core/debugfs.c:75:7: warning: format specifies type 'unsigned long long' but the argument has type 'dma_addr_t' (aka 'unsigned int') [-Wformat] mhi_event->db_cfg.db_val); ^~~~ drivers/bus/mhi/core/debugfs.c:123:7: warning: format specifies type 'unsigned long long' but the argument has type 'dma_addr_t' (aka 'unsigned int') [-Wformat] mhi_chan->db_cfg.db_val); ^~~ 2 warnings generated. drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_events_show’: drivers/bus/mhi/core/debugfs.c:74:51: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] seq_printf(m, " local rp: 0x%llx db: 0x%pad\n", (u64)ring->rp, ^ drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_channels_show’: drivers/bus/mhi/core/debugfs.c:122:7: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] (u64)ring->rp, (u64)ring->wp, ^ drivers/bus/mhi/core/debugfs.c:122:22: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] (u64)ring->rp, (u64)ring->wp, ^ drivers/bus/mhi/core/debugfs.c:121:62: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 5 has type ‘dma_addr_t {aka unsigned int}’ [-Wformat=] seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", ~~~^ %x drivers/bus/mhi/core/debugfs.c:123:7: mhi_chan->db_cfg.db_val); Reported-by: kernel test robot Signed-off-by: Manivannan Sadhasivam --- Greg: This fixes the issue seen while testing the char-misc/char-misc-testing branch. drivers/bus/mhi/core/debugfs.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/bus/mhi/core/debugfs.c b/drivers/bus/mhi/core/debugfs.c index 53d05a8e168d..2536ff92b76f 100644 --- a/drivers/bus/mhi/core/debugfs.c +++ b/drivers/bus/mhi/core/debugfs.c @@ -71,8 +71,8 @@ static int mhi_debugfs_events_show(struct seq_file *m, void *d) seq_printf(m, " rp: 0x%llx wp: 0x%llx", er_ctxt->rp, er_ctxt->wp); - seq_printf(m, " local rp: 0x%llx db: 0x%llx\n", (u64)ring->rp, - mhi_event->db_cfg.db_val); + seq_printf(m, " local rp: 0x%px db: 0x%pad\n", ring->rp, + &mhi_event->db_cfg.db_val); } return 0; @@ -118,9 +118,9 @@ static int mhi_debugfs_channels_show(struct seq_file *m, void *d) seq_printf(m, " base: 0x%llx len: 0x%llx wp: 0x%llx", chan_ctxt->rbase, chan_ctxt->rlen, chan_ctxt->wp); - seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", - (u64)ring->rp, (u64)ring->wp, - mhi_chan->db_cfg.db_val); + seq_printf(m, " local rp: 0x%px local wp: 0x%px db: 0x%pad\n", + ring->rp, ring->wp, + &mhi_chan->db_cfg.db_val); } return 0; Documentation/printk-formats.txt seems to point out that %px is "insecure" and thus perhaps not preferred. Are we assuming that debugfs is only accessible by root, and thus the %px usage here is effectively the same as %pK? -- Jeffrey Hugo Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH] bus: mhi: core: debugfs: Use correct format specifiers for addresses
For exposing the addresses of read/write pointers and doorbell register, let's use the correct format specifiers. This fixes the following issues generated using W=1 build in ARM32 and reported by Kbuild bot: All warnings (new ones prefixed by >>): >> drivers/bus/mhi/core/debugfs.c:75:7: warning: format specifies type >> 'unsigned long long' but the argument has type 'dma_addr_t' (aka 'unsigned >> int') [-Wformat] mhi_event->db_cfg.db_val); ^~~~ drivers/bus/mhi/core/debugfs.c:123:7: warning: format specifies type 'unsigned long long' but the argument has type 'dma_addr_t' (aka 'unsigned int') [-Wformat] mhi_chan->db_cfg.db_val); ^~~ 2 warnings generated. drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_events_show’: drivers/bus/mhi/core/debugfs.c:74:51: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] seq_printf(m, " local rp: 0x%llx db: 0x%pad\n", (u64)ring->rp, ^ drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_channels_show’: drivers/bus/mhi/core/debugfs.c:122:7: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] (u64)ring->rp, (u64)ring->wp, ^ drivers/bus/mhi/core/debugfs.c:122:22: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] (u64)ring->rp, (u64)ring->wp, ^ drivers/bus/mhi/core/debugfs.c:121:62: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 5 has type ‘dma_addr_t {aka unsigned int}’ [-Wformat=] seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", ~~~^ %x drivers/bus/mhi/core/debugfs.c:123:7: mhi_chan->db_cfg.db_val); Reported-by: kernel test robot Signed-off-by: Manivannan Sadhasivam --- Greg: This fixes the issue seen while testing the char-misc/char-misc-testing branch. drivers/bus/mhi/core/debugfs.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/bus/mhi/core/debugfs.c b/drivers/bus/mhi/core/debugfs.c index 53d05a8e168d..2536ff92b76f 100644 --- a/drivers/bus/mhi/core/debugfs.c +++ b/drivers/bus/mhi/core/debugfs.c @@ -71,8 +71,8 @@ static int mhi_debugfs_events_show(struct seq_file *m, void *d) seq_printf(m, " rp: 0x%llx wp: 0x%llx", er_ctxt->rp, er_ctxt->wp); - seq_printf(m, " local rp: 0x%llx db: 0x%llx\n", (u64)ring->rp, - mhi_event->db_cfg.db_val); + seq_printf(m, " local rp: 0x%px db: 0x%pad\n", ring->rp, + &mhi_event->db_cfg.db_val); } return 0; @@ -118,9 +118,9 @@ static int mhi_debugfs_channels_show(struct seq_file *m, void *d) seq_printf(m, " base: 0x%llx len: 0x%llx wp: 0x%llx", chan_ctxt->rbase, chan_ctxt->rlen, chan_ctxt->wp); - seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n", - (u64)ring->rp, (u64)ring->wp, - mhi_chan->db_cfg.db_val); + seq_printf(m, " local rp: 0x%px local wp: 0x%px db: 0x%pad\n", + ring->rp, ring->wp, + &mhi_chan->db_cfg.db_val); } return 0; -- 2.17.1