On 05-Jan-19 19:41, Jason Gunthorpe wrote: > On Sat, Jan 05, 2019 at 07:23:32PM +0200, Gal Pressman wrote: > >>> Oh? EFA is sharing BAR pages between user processes? You have a >>> security proof that is OK? >> >> I guess we're talking about PDs? > > In most devices available BAR address space is the limit to > ucontexts.. In our case the BAR space limits the number of PDs. Each PD is reserved with its own BAR address space, it is guaranteed that all QPs doorbells of a specific PD will reside on this address space. The pages are not being shared. > >> There's a PD limit (currently 128, depends on the device) which >> limits the number of processes. There is no sharing of BAR pages >> between user processes. > > EFA has some design problems here.. Generally mapping of a BAR page into > user space must be done under the ucontext, not for individual > objects. ie if I allocate BAR page X to ucontext Y then X must remain > allocated until ucontext Y is destroyed. When you say alloc BAR page do you mean mmap BAR page? We map the pages for individual objects, but the whole BAR is already "allocated" for the associated PD. The mapping stays valid until the userspace calls munmap. > > Otherwise there can be use-after free style security bugs. > > Since the efa_dealloc_ucontext does nothing, and BAR pages are being > mapped, it must be wrong. Can you please elaborate? what would you like to see in dealloc_ucontext? > > It kind of looks like it is trying to tie BAR allocation lifetime to > individual objects? > > .. and all of this is why one generally focuses on the ucontext as the > limit, as generally, allocating a ucontext implies allocating a BAR > page, and thus the number of ucontexts is strictly limited by the BAR > size. s/ucontext/PD/g is the case for EFA, our device is not aware of ucontext but PDs. The BAR "reservation" is there for the lifetime of the PD. > > A driver can do lazy allocation, but it is kind of pointless. > > Jason >
