On Thu, Feb 20, 2025 at 8:22 AM Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote: > > On Thu, Feb 20, 2025 at 5:18 AM Lorenzo Stoakes > <lorenzo.stoakes@xxxxxxxxxx> wrote: > > > > On Thu, Feb 20, 2025 at 01:44:20PM +0100, David Hildenbrand wrote: > > > On 20.02.25 11:15, Lorenzo Stoakes wrote: > > > > On Thu, Feb 20, 2025 at 11:03:02AM +0100, David Hildenbrand wrote: > > > > > > > Your conclusion is 'did not participate with upstream'; I don't agree with > > > > > > > that. But maybe you and Kalesh have a history on that that let's you react > > > > > > > on his questions IMHO more emotionally than it should have been. > > > > > > > > > > > > This is wholly unfair, I have been very reasonable in response to this > > > > > > thread. I have offered to find solutions, I have tried to understand the > > > > > > problem in spite of having gone to great lengths to try to discuss the > > > > > > limitations of the proposed approach in every venue I possibly could. > > > > > > > > > > > > I go out of my way to deal professionally and objectively with what is > > > > > > presented. Nothing here is emotional. So I'd ask that you please abstain > > > > > > from making commentary like this which has no basis. > > > > > > > > > > I appreciate everything you write below. But this request is just > > > > > impossible. I will keep raising my opinion and misunderstandings will > > > > > happen. > > > > > > > > Well I wouldn't ask you not to express your opinion David, you know I respect > > > > and like you, and by all means push back hard or call out what you think is bad > > > > behaviour :) > > > > > > > > I just meant to say, in my view, that there was no basis, but I appreciate > > > > miscommunications happen. > > > > > So apologies if I came off as being difficult or rude, it actually > > > wasn't > > > > intended. And to re-emphasise - I have zero personal issue with anybody in this > > > > thread whatsoever! > > > > > > It sounded to me like you were trying to defend your work (again, IMHO too > > > emotionally, and I might have completely misinterpreted that) and slowly > > > switching to "friendly fire" (towards me). Apologies from my side if I > > > completely misunderstood/misinterpreted that. > > > > Right this was not at all my intent, sorry if it seemed that way. I may well > > have communicated terribly, so apologies on my side too. Hi everyone, Thank you for all the discussion. I don't find any personal issues with the communication in this thread, but I appreciate David being the object voice of reason. I understand it can be frustrating since you have made many efforts to communicate these tradeoffs. Unfortunately these issues were not known for the file-backed ELF guard regions for my particular use case. > > Sorry for being late to the party. Was sick for a couple of days. > Lorenzo is right, there was a breakdown in communication at Google and > he has all the rights to be upset. The issue with obfuscators should > have been communicated once it was discovered. I was in regular > discussions with Lorenzo but wasn't directly involved with this > particular project and wasn't aware or did not realize that the > obfuscator issue renders guards unusable for this usecase. My > apologies, I should have asked more questions about it. I suspect > Lorenzo would have implemented this anyway... > Suren's use case is different from mine and this design fits perfectly for anon guard regions from the allocator. :) So I think in conclusion, these aren't VMAs and shouldn't be treated as such; we will advertise them from pagemap for those who need to know. -- Kalesh > To make guard regions work for this usecase, first we (Android) need > to abstract /proc/pid/maps accesses. Only then we can use additional > interfaces like /proc/pid/pagemaps to obtain guard region information. > I'll start figuring out what it takes to insert such an abstraction. > Thanks, > Suren. > > > > > > > > > > To recap: what we have upstream is great; you did a great job. Yes, the > > > mechanism has its drawbacks, but that's just part of the design. > > > > Thanks :) > > > > > > > > Some people maybe have wrong expectations, maybe there were > > > misunderstandings, or maybe there are requirements that only now pop up; > > > it's sometimes unavoidable, and that's ok. > > > > > > We can try to document it better (and I was trying to find clues why people > > > might be mislead), and see if/how we could sort out these requirements. But > > > we can likely not make it perfect in any possible way (I'm sure there are > > > plenty of use cases where what we currently have is more than sufficient). > > > > Sure and I"m very open to adding a documentation page for guard regions, in > > fact was considering this very thing recently. I already added man pages > > but be good to be able to go into more depth. > > > > > > > > > > I just want to find the best way forward, technically and am willing to > > > do > > > > whatever work is required to make the guard region implementation as good as it > > > > possibly can be. > > > > > > > > > > > > > > Note that the whole "Honestly David you and the naming. .." thing could have > > > > > been written as "I don't think it's a naming problem." > > > > > > > > I feel like I _always_ get in trouble when I try to write in a 'tongue-in-cheek' > > > > style, which is what this was meant to be... so I think herein lies the basis of > > > > the miscommunication :) > > > > > > > > I apologise, the household is ill, which maybe affects my judgment in how I > > > > write these, but in general text is a very poor medium. It was meant to be said > > > > in a jolly tone with a wink... > > > > > > > > I think maybe I should learn my lesson with these things, I thought the ':p' > > > > would make this clear but yeah, text, poor medium. > > > > > > > > Anyway apologies if this seemed disrespectful. > > > > > > No worries, it's hard to really make me angry, and I appreciate your > > > openness and your apology (well, and you and your work, obviously). > > > > > > I'll note, though, if my memory serves me right, that nobody so far ever > > > criticized the way I communicate upstream, or even told me to abstain from > > > certain communication. > > > > I wish I could say the same haha, so perhaps this was a problem on my side > > honestly. I do have a habit of being 'tongue in cheek' and failing to > > communicate that which I did say the last time I wouldn't repeat. It is not > > intended, I promise. > > > > As the abstain, was more a British turn of phrase, meaning to say - I > > dispute the claim that this is an emotional thing and please don't say this > > if it isn't so. > > > > But I understand that of course, you may have interpreted it as so, due to > > my having failed to communicate it well. > > > > Again, I must say, text remains replete with possibilities for > > miscommunication, misunderstanding and it can so often be difficult to > > communicate one's intent. > > > > But again of course, I apologise if I overstepped the line in any way! > > > > > > > > That probably hurt most, now that a couple of hours passed. Nothing that a > > > couple of beers and a bit of self-reflection on my communication style can't > > > fix ;) > > > > Ugh sorry, man. Not my intent. And it seems - I literally OWE YOU pints > > now. :) we will fix this at lsf... > > > > Perhaps owe Kalesh some too should he be there... will budget > > accordingly... :P > > > > > > > > [...] > > > > > > > > > > > Yeah that's a good point, but honestly if you're reading smaps that reads > > > > > > > > the page tables, then reading /proc/$pid/pagemaps and reading page tables > > > > > > > > TWICE that seems inefficient vs. just reading /proc/$pid/maps, then reading > > > > > > > > /proc/$pid/pagemaps and reading page tables once. > > > > > > > > > > > > > > Right; I recently wished that we would have an interface to obtain more VMA > > > > > > > flags without having to go through smaps > > > > > > > > > > > > Well maybe that lends itself to the idea of adding a whole new interface in > > > > > > general... > > > > > > > > > > An extended "maps" interface might be reasonable, that allows for exposing > > > > > more things without walking the page tables. (e.g., flags) > > > > > > > > > > Maybe one could have an indicator that says "ever had guard regions in this > > > > > mapping" without actually walking the page tables. > > > > > > > > Yeah this is something we've discussed before, but it's a little fraught. Let's > > > > say it was a VMA flag, in this case we'd have to make this flag 'sticky' and not > > > > impact merging (easy enough) to account for splits/merges. > > > > > The problem comes in that we would then need to acquire the VMA write > > > lock to do > > > > it, something we don't currently require on application of guard regions. > > > > > > Right, and we shouldn't write-lock the mmap. We'd need some way to just > > > atomically set such an indicator on a VMA. > > > > Hm yeah, could be tricky, we definitely can't manage a new field in > > vm_area_struct, this is a very sensitive subject at the moment really with > > Suren's work with VMAs allocated via SLAB_TYPESAFE_BY_RCU, putting the lock > > into the VMA and the alignment requirements. > > > > Not sure what precedent we'd have with atomic setting of a VMA flag for > > this... could be tricky. > > > > > > > > I'll also note that it might be helpful for smallish region, but especially > > > for large ones (including when they are split and the indicator is wrong), > > > it's less helpful. I don't have to tell you about the VMA merging > > > implications, probably it would be like VM_SOFTDIRTY handling :) > > > > Yeah indeed now we've simplified merging a lot of possibilities emerge, > > this is one! > > > > > > > > > > > > > We'd also have to make sure nothing else makes any assumptions about VMA flags > > > > implying differences in VMAs in this one instance (though we do already do this > > > > for VM_SOFTDIRTY). > > > > > > > > I saw this as possibly something like VM_MAYBE_GUARD_REGIONS or something. > > > > > > Yes. > > > > > > -- > > > Cheers, > > > > > > David / dhildenb > > > > > > > Best, Lorenzo
