On 7/27/23 4:26 PM, Michał Mirosław wrote: > On Thu, 27 Jul 2023 at 10:03, Muhammad Usama Anjum > <usama.anjum@xxxxxxxxxxxxx> wrote: >> On 7/27/23 2:10 AM, Michał Mirosław wrote: >>> On Wed, 26 Jul 2023 at 10:34, Muhammad Usama Anjum >>> <usama.anjum@xxxxxxxxxxxxx> wrote: >>>> On 7/25/23 11:05 PM, Michał Mirosław wrote: >>>>> On Tue, 25 Jul 2023 at 11:11, Muhammad Usama Anjum >>>>> <usama.anjum@xxxxxxxxxxxxx> wrote: > [...] >>>>> 2. For the address tagging part I'd prefer someone who knows how this >>>>> is used take a look. We're ignoring the tag (but clear it on return in >>>>> ->start) - so it doesn't matter for the ioctl() itself. >>>> I've added Kirill if he can give his thoughts about tagged memory. >>>> >>>> Right now we are removing the tags from all 3 pointers (start, end, vec) >>>> before using the pointers on kernel side. But we are overwriting and >>>> writing the walk ending address in start which user can read/use. >>>> >>>> I think we shouldn't over-write the start (and its tag) and instead return >>>> the ending walk address in new variable, walk_end. >>> >>> The overwrite of `start` is making the ioctl restart (continuation) >>> easier to handle. I prefer the current way, but it's not a strong >>> opinion. >> We shouldn't overwrite the start if we aren't gonna put the correct tag. So >> I've resorted to adding another variable `walk_end` to return the walk >> ending address. > > Yes. We have two options: > > 1. add new field and have the userspace check it and update start > itself to continue the scan, I've selected this option and sent v26 already: https://lore.kernel.org/all/20230727093637.1262110-1-usama.anjum@xxxxxxxxxxxxx > or: > 2. reconstruct the tag from either orignal `start` or `end` and have > the userspace re-set `start` if it wants to restart the scan instead > of continuing. In some case, compiler can put integrity checking metadata in the pointer's upper byte. So copying start or end's meta data would be wrong. > > (the second one, using `end`'s tag, might be the easiest for > userspace, as it can check `start` == `end` when deciding to continue > or restart). > > Best Regards > Michał Mirosław -- BR, Muhammad Usama Anjum
