On 8/12/19 12:39 PM, Suman Anna wrote: > On 8/12/19 11:36 AM, Andrew F. Davis wrote: >> On 8/12/19 12:28 PM, Suman Anna wrote: >>> On 8/12/19 10:47 AM, Andrew F. Davis wrote: >>>> On 10/23/18 9:19 PM, Suman Anna wrote: >>>>> The virtio_rpmsg_bus driver uses the "%p" format-specifier for >>>>> printing the vring buffer address. This prints only a hashed >>>>> pointer even for previliged users. Use "%pK" instead so that >>>>> the address can be printed during debug using kptr_restrict >>>>> sysctl. >>>>> >>>> >>>> >>>> s/previliged/privileged >>> >>> Bjorn, >>> Can you fix this up when applying. >>> >>>> >>>> You describe what the code does, but not why you need this. %pK is used >>>> for only about 1% of pointer printing, why do you want to leak this >>>> address to userspace at all? >>> >>> Andrew, >>> Default behavior of %pK is same as %p, but it does allow you to control >>> the print. The reason is clearly mentioned in the last sentence in the >>> patch description. >>> >> >> >> Let me rephrase then, why would you ever set 'kptr_restrict' to anything >> other than 0, or why do you want to be able to leak this address to >> userspace at all? If the answer is just because you can, then all 12,000 >> instances of %p in kernel could be converted for the same reason. > > It is a dev_dbg statement, so it is already under dynamic debug control. > We would only ever use it during debug. > Most pointer printings are in debug statements.. I'm simply not seeing what this helps us do. The DMA address I can understand, it may be given to a remote core so we may want to verify it is the same on both sides, but the actual virtual kernel address is of no value to us, a hash to track it across uses is just as good. Andrew > regards > Suman > >> >> Andrew >> >> >>> regards >>> Suman >>> >>>> >>>> Andrew >>>> >>>> >>>>> Signed-off-by: Suman Anna <s-anna@xxxxxx> >>>>> --- >>>>> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c >>>>> index f29dee731026..1345f373a1a0 100644 >>>>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c >>>>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c >>>>> @@ -950,7 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev) >>>>> goto vqs_del; >>>>> } >>>>> >>>>> - dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", >>>>> + dev_dbg(&vdev->dev, "buffers: va %pK, dma %pad\n", >>>>> bufs_va, &vrp->bufs_dma); >>>>> >>>>> /* half of the buffers is dedicated for RX */ >>>>> >>> >
