From: James Bottomley <hidden> Date: 2021-02-01 16:58:23
On Fri, 2021-01-29 at 09:23 +0100, Michal Hocko wrote:
On Thu 28-01-21 13:05:02, James Bottomley wrote:
quoted
Obviously the API choice could be revisited
but do you have anything to add over the previous discussion, or is
this just to get your access control?
Well, access control is certainly one thing which I still believe is
missing. But if there is a general agreement that the direct map
manipulation is not that critical then this will become much less of
a problem of course.
The secret memory is a scarce resource but it's not a facility that
should only be available to some users.
It all boils down whether secret memory is a scarce resource. With
the existing implementation it really is. It is effectivelly
repeating same design errors as hugetlb did. And look now, we have a
subtle and convoluted reservation code to track mmap requests and we
have a cgroup controller to, guess what, have at least some control
over distribution if the preallocated pool. See where am I coming
from?
I'm fairly sure rlimit is the correct way to control this. The
subtlety in both rlimit and memcg tracking comes from deciding to
account under an existing category rather than having our own new one.
People don't like new stuff in accounting because it requires
modifications to everything in userspace. Accounting under and
existing limit keeps userspace the same but leads to endless arguments
about which limit it should be under. It took us several patch set
iterations to get to a fragile consensus on this which you're now
disrupting for reasons you're not making clear.
If the secret memory is more in line with mlock without any imposed
limit (other than available memory) in the end then, sure, using the
same access control as mlock sounds reasonable. Btw. if this is
really just a more restrictive mlock then is there any reason to not
hook this into the existing mlock infrastructure (e.g.
MCL_EXCLUSIVE)? Implications would be that direct map would be
handled on instantiation/tear down paths, migration would deal with
the same (if possible). Other than that it would be mlock like.
In the very first patch set we proposed a mmap flag to do this. Under
detailed probing it emerged that this suffers from several design
problems: the KVM people want VMM to be able to remove the secret
memory range from the process; there may be situations where sharing is
useful and some people want to be able to seal the operations. All of
this ended up convincing everyone that a file descriptor based approach
was better than a mmap one.
James
From: Michal Hocko <mhocko@suse.com> Date: 2021-02-02 09:37:47
On Mon 01-02-21 08:56:19, James Bottomley wrote:
On Fri, 2021-01-29 at 09:23 +0100, Michal Hocko wrote:
quoted
On Thu 28-01-21 13:05:02, James Bottomley wrote:
quoted
Obviously the API choice could be revisited
but do you have anything to add over the previous discussion, or is
this just to get your access control?
Well, access control is certainly one thing which I still believe is
missing. But if there is a general agreement that the direct map
manipulation is not that critical then this will become much less of
a problem of course.
The secret memory is a scarce resource but it's not a facility that
should only be available to some users.
How those two objectives go along? Or maybe our understanding of what
scrace really means here. If the pool of the secret memory is very limited
then you really need a way to stop one party from depriving others. More
on that below.
quoted
It all boils down whether secret memory is a scarce resource. With
the existing implementation it really is. It is effectivelly
repeating same design errors as hugetlb did. And look now, we have a
subtle and convoluted reservation code to track mmap requests and we
have a cgroup controller to, guess what, have at least some control
over distribution if the preallocated pool. See where am I coming
from?
I'm fairly sure rlimit is the correct way to control this. The
subtlety in both rlimit and memcg tracking comes from deciding to
account under an existing category rather than having our own new one.
People don't like new stuff in accounting because it requires
modifications to everything in userspace. Accounting under and
existing limit keeps userspace the same but leads to endless arguments
about which limit it should be under. It took us several patch set
iterations to get to a fragile consensus on this which you're now
disrupting for reasons you're not making clear.
I hoped I had made my points really clear. The existing scheme allows
one users (potentially adversary) to deplete the preallocated pool
and cause a shitstorm of OOM killer because there is no real way to
replenish the pool from the oom killer other than randomly keep killing
tasks until one happens to release its secret memory back to the
pool. Is that more clear now?
And no, rlimit and memcg limit will not save you from that because the
former is per process and later is hard to manage under a single limit
which might be order of magnitude larger than the secret memory pool
size. See the point?
I have also proposed potential ways out of this. Either the pool is not
fixed sized and you make it a regular unevictable memory (if direct map
fragmentation is not considered a major problem) or you need a careful
access control or you need SIGBUS on the mmap failure (to allow at least
some fallback mode to caller).
I do not see any other way around it. I might be missing some other
ways but so far I keep hearing that the existing scheme is just fine
because this has been discussed in the past and you have agreed it is
ok. Without any specifics...
Please keep in mind this is a user interface and it is due to careful
scrutiny. So rather than pushing back with "you are disrupting a
consensus" kinda feedback, please try to stay technical.
quoted
If the secret memory is more in line with mlock without any imposed
limit (other than available memory) in the end then, sure, using the
same access control as mlock sounds reasonable. Btw. if this is
really just a more restrictive mlock then is there any reason to not
hook this into the existing mlock infrastructure (e.g.
MCL_EXCLUSIVE)? Implications would be that direct map would be
handled on instantiation/tear down paths, migration would deal with
the same (if possible). Other than that it would be mlock like.
In the very first patch set we proposed a mmap flag to do this. Under
detailed probing it emerged that this suffers from several design
problems: the KVM people want VMM to be able to remove the secret
memory range from the process; there may be situations where sharing is
useful and some people want to be able to seal the operations. All of
this ended up convincing everyone that a file descriptor based approach
was better than a mmap one.
OK, fair enough. This belongs to the changelog IMHO. It is good to know
why existing interfaces do not match the need.
--
Michal Hocko
SUSE Labs
From: Mike Rapoport <rppt@kernel.org> Date: 2021-02-02 12:50:23
On Tue, Feb 02, 2021 at 10:35:05AM +0100, Michal Hocko wrote:
On Mon 01-02-21 08:56:19, James Bottomley wrote:
I have also proposed potential ways out of this. Either the pool is not
fixed sized and you make it a regular unevictable memory (if direct map
fragmentation is not considered a major problem)
I think that the direct map fragmentation is not a major problem, and the
data we have confirms it, so I'd be more than happy to entirely drop the
pool, allocate memory page by page and remove each page from the direct
map.
Still, we cannot prove negative and it could happen that there is a
workload that would suffer a lot from the direct map fragmentation, so
having a pool of large pages upfront is better than trying to fix it
afterwards. As we get more confidence that the direct map fragmentation is
not an issue as it is common to believe we may remove the pool altogether.
I think that using PMD_ORDER allocations for the pool with a fallback to
order 0 will do the job, but unfortunately I doubt we'll reach a consensus
about this because dogmatic beliefs are hard to shake...
A more restrictive possibility is to still use plain PMD_ORDER allocations
to fill the pool, without relying on CMA. In this case there will be no
global secretmem specific pool to exhaust, but then it's possible to drain
high order free blocks in a system, so CMA has an advantage of limiting
secretmem pools to certain amount of memory with somewhat higher
probability for high order allocation to succeed.
or you need a careful access control
Do you mind elaborating what do you mean by "careful access control"?
or you need SIGBUS on the mmap failure (to allow at least some fallback
mode to caller).
As I've already said, I agree that SIGBUS is way better than OOM at #PF
time.
And we can add some means to fail at mmap() time if the pools are running
low.
--
Sincerely yours,
Mike.
From: David Hildenbrand <hidden> Date: 2021-02-02 13:18:36
On 02.02.21 13:48, Mike Rapoport wrote:
On Tue, Feb 02, 2021 at 10:35:05AM +0100, Michal Hocko wrote:
quoted
On Mon 01-02-21 08:56:19, James Bottomley wrote:
I have also proposed potential ways out of this. Either the pool is not
fixed sized and you make it a regular unevictable memory (if direct map
fragmentation is not considered a major problem)
I think that the direct map fragmentation is not a major problem, and the
data we have confirms it, so I'd be more than happy to entirely drop the
pool, allocate memory page by page and remove each page from the direct
map.
Still, we cannot prove negative and it could happen that there is a
workload that would suffer a lot from the direct map fragmentation, so
having a pool of large pages upfront is better than trying to fix it
afterwards. As we get more confidence that the direct map fragmentation is
not an issue as it is common to believe we may remove the pool altogether.
I think that using PMD_ORDER allocations for the pool with a fallback to
order 0 will do the job, but unfortunately I doubt we'll reach a consensus
about this because dogmatic beliefs are hard to shake...
A more restrictive possibility is to still use plain PMD_ORDER allocations
to fill the pool, without relying on CMA. In this case there will be no
global secretmem specific pool to exhaust, but then it's possible to drain
high order free blocks in a system, so CMA has an advantage of limiting
secretmem pools to certain amount of memory with somewhat higher
probability for high order allocation to succeed.
I am not really concerned about fragmenting/breaking up the direct map
as long as the feature has to be explicitly enabled (similar to
fragmenting the vmemmap).
As already expressed, I dislike allowing user space to consume an
unlimited number unmovable/unmigratable allocations. We already have
that in some cases with huge pages (when the arch does not support
migration) - but there we can at least manage the consumption using the
whole max/reserved/free/... infrastructure. In addition, adding arch
support for migration shouldn't be too complicated.
The idea of using CMA is quite good IMHO, because there we can locally
limit the direct map fragmentation and don't have to bother about
migration at all. We own the area, so we can place as many unmovable
allocations on it as we can fit.
But it sounds like, we would also need some kind of reservation
mechanism in either scenario (CMA vs. no CMA).
If we don't want to go full-circle on max/reserved/free/..., allowing
for migration of secretmem pages would make sense. Then, these pages
become "less special". Map source, copy, unmap destination. The security
implementations are the ugly part. I wonder if we could temporarily map
somewhere else, so avoiding to touch the direct map during migration.
--
Thanks,
David / dhildenb
From: Michal Hocko <mhocko@suse.com> Date: 2021-02-02 13:28:27
On Tue 02-02-21 14:48:57, Mike Rapoport wrote:
On Tue, Feb 02, 2021 at 10:35:05AM +0100, Michal Hocko wrote:
quoted
On Mon 01-02-21 08:56:19, James Bottomley wrote:
I have also proposed potential ways out of this. Either the pool is not
fixed sized and you make it a regular unevictable memory (if direct map
fragmentation is not considered a major problem)
I think that the direct map fragmentation is not a major problem, and the
data we have confirms it, so I'd be more than happy to entirely drop the
pool, allocate memory page by page and remove each page from the direct
map.
Still, we cannot prove negative and it could happen that there is a
workload that would suffer a lot from the direct map fragmentation, so
having a pool of large pages upfront is better than trying to fix it
afterwards. As we get more confidence that the direct map fragmentation is
not an issue as it is common to believe we may remove the pool altogether.
I would drop the pool altogether and instantiate pages to the
unevictable LRU list and internally treat it as ramdisk/mlock so you
will get an accounting correctly. The feature should be still opt-in
(e.g. a kernel command line parameter) for now. The recent report by
Intel (http://lkml.kernel.org/r/213b4567-46ce-f116-9cdf-bbd0c884eb3c@linux.intel.com)
there is no clear win to have huge mappings in _general_ but there are
still workloads which benefit.
I think that using PMD_ORDER allocations for the pool with a fallback to
order 0 will do the job, but unfortunately I doubt we'll reach a consensus
about this because dogmatic beliefs are hard to shake...
If this is opt-in then those beliefs can be relaxed somehow. Long term
it makes a lot of sense to optimize for a better direct map management
but I do not think this is a hard requirement for an initial
implementation if it is not imposed to everybody by default.
A more restrictive possibility is to still use plain PMD_ORDER allocations
to fill the pool, without relying on CMA. In this case there will be no
global secretmem specific pool to exhaust, but then it's possible to drain
high order free blocks in a system, so CMA has an advantage of limiting
secretmem pools to certain amount of memory with somewhat higher
probability for high order allocation to succeed.
quoted
or you need a careful access control
Do you mind elaborating what do you mean by "careful access control"?
As already mentioned, a mechanism to control who can use this feature -
e.g. make it a special device which you can access control by
permissions or higher level security policies. But that is really needed
only if the pool is fixed sized.
quoted
or you need SIGBUS on the mmap failure (to allow at least some fallback
mode to caller).
As I've already said, I agree that SIGBUS is way better than OOM at #PF
time.
It would be better than OOM but it would still be a terrible interface.
So I would go that path only as a last resort. I do not even want to
think what kind of security consequences that would have. E.g. think of
somebody depleting the pool and pushing security sensitive workload into
fallback which is not backed by security memory.
And we can add some means to fail at mmap() time if the pools are running
low.
Welcome to hugetlb reservation world...
--
Michal Hocko
SUSE Labs
From: Michal Hocko <mhocko@suse.com> Date: 2021-02-02 13:34:53
On Tue 02-02-21 14:14:09, David Hildenbrand wrote:
[...]
As already expressed, I dislike allowing user space to consume an unlimited
number unmovable/unmigratable allocations. We already have that in some
cases with huge pages (when the arch does not support migration) - but there
we can at least manage the consumption using the whole max/reserved/free/...
infrastructure. In addition, adding arch support for migration shouldn't be
too complicated.
Well, mlock is not too different here as well. Hugepages are arguably an
easier model because it requires an explicit pre-configuration by an
admin. Mlock doesn't have anything like that. Please also note that
while mlock pages are migrateable by default, this is not the case in
general because they can be configured to disalow migration to prevent
from minor page faults as some workloads require that (e.g. RT).
Another example is ramdisk or even tmpfs (with swap storage depleted or
not configured). Both are PITA from the OOM POV but they are manageable
if people are careful. If secretmem behaves along those existing models
then we know what to expect at least.
--
Michal Hocko
SUSE Labs
From: David Hildenbrand <hidden> Date: 2021-02-02 14:16:34
On 02.02.21 14:32, Michal Hocko wrote:
On Tue 02-02-21 14:14:09, David Hildenbrand wrote:
[...]
quoted
As already expressed, I dislike allowing user space to consume an unlimited
number unmovable/unmigratable allocations. We already have that in some
cases with huge pages (when the arch does not support migration) - but there
we can at least manage the consumption using the whole max/reserved/free/...
infrastructure. In addition, adding arch support for migration shouldn't be
too complicated.
Well, mlock is not too different here as well. Hugepages are arguably an
easier model because it requires an explicit pre-configuration by an
admin. Mlock doesn't have anything like that. Please also note that
while mlock pages are migrateable by default, this is not the case in
general because they can be configured to disalow migration to prevent
from minor page faults as some workloads require that (e.g. RT).
Yeah, however that is a very special case. In most cases mlock() simply
prevents swapping, you still have movable pages you can place anywhere
you like (including on ZONE_MOVABLE).
Another example is ramdisk or even tmpfs (with swap storage depleted or
not configured). Both are PITA from the OOM POV but they are manageable
if people are careful.
Right, but again, special cases - e.g., tmpfs explicitly has to be resized.
If secretmem behaves along those existing models
then we know what to expect at least.
I think secretmem behaves much more like longterm GUP right now
("unmigratable", "lifetime controlled by user space", "cannot go on
CMA/ZONE_MOVABLE"). I'd either want to reasonably well control/limit it
or make it behave more like mlocked pages.
--
Thanks,
David / dhildenb
From: Michal Hocko <mhocko@suse.com> Date: 2021-02-02 14:25:53
On Tue 02-02-21 15:12:21, David Hildenbrand wrote:
[...]
I think secretmem behaves much more like longterm GUP right now
("unmigratable", "lifetime controlled by user space", "cannot go on
CMA/ZONE_MOVABLE"). I'd either want to reasonably well control/limit it or
make it behave more like mlocked pages.
I thought I have already asked but I must have forgotten. Is there any
actual reason why the memory is not movable? Timing attacks?
--
Michal Hocko
SUSE Labs
From: David Hildenbrand <hidden> Date: 2021-02-02 14:29:12
On 02.02.21 15:22, Michal Hocko wrote:
On Tue 02-02-21 15:12:21, David Hildenbrand wrote:
[...]
quoted
I think secretmem behaves much more like longterm GUP right now
("unmigratable", "lifetime controlled by user space", "cannot go on
CMA/ZONE_MOVABLE"). I'd either want to reasonably well control/limit it or
make it behave more like mlocked pages.
I thought I have already asked but I must have forgotten. Is there any
actual reason why the memory is not movable? Timing attacks?
I think the reason is simple: no direct map, no copying of memory.
As I mentioned, we would have to temporarily map in order to copy.
Mapping it somewhere else (like kmap), outside of the direct map might
reduce possible attacks.
--
Thanks,
David / dhildenb
From: Michal Hocko <mhocko@suse.com> Date: 2021-02-02 14:34:14
On Tue 02-02-21 15:26:20, David Hildenbrand wrote:
On 02.02.21 15:22, Michal Hocko wrote:
quoted
On Tue 02-02-21 15:12:21, David Hildenbrand wrote:
[...]
quoted
I think secretmem behaves much more like longterm GUP right now
("unmigratable", "lifetime controlled by user space", "cannot go on
CMA/ZONE_MOVABLE"). I'd either want to reasonably well control/limit it or
make it behave more like mlocked pages.
I thought I have already asked but I must have forgotten. Is there any
actual reason why the memory is not movable? Timing attacks?
I think the reason is simple: no direct map, no copying of memory.
This is an implementation detail though and not something terribly hard
to add on top later on. I was more worried there would be really
fundamental reason why this is not possible. E.g. security implications.
--
Michal Hocko
SUSE Labs
From: David Hildenbrand <hidden> Date: 2021-02-02 14:47:40
On 02.02.21 15:32, Michal Hocko wrote:
On Tue 02-02-21 15:26:20, David Hildenbrand wrote:
quoted
On 02.02.21 15:22, Michal Hocko wrote:
quoted
On Tue 02-02-21 15:12:21, David Hildenbrand wrote:
[...]
quoted
I think secretmem behaves much more like longterm GUP right now
("unmigratable", "lifetime controlled by user space", "cannot go on
CMA/ZONE_MOVABLE"). I'd either want to reasonably well control/limit it or
make it behave more like mlocked pages.
I thought I have already asked but I must have forgotten. Is there any
actual reason why the memory is not movable? Timing attacks?
I think the reason is simple: no direct map, no copying of memory.
This is an implementation detail though and not something terribly hard
to add on top later on. I was more worried there would be really
fundamental reason why this is not possible. E.g. security implications.
I don't remember all the details. Let's see what Mike thinks regarding
migration (e.g., security concerns).
--
Thanks,
David / dhildenb
From: Mike Rapoport <rppt@kernel.org> Date: 2021-02-02 18:19:37
On Tue, Feb 02, 2021 at 03:34:29PM +0100, David Hildenbrand wrote:
On 02.02.21 15:32, Michal Hocko wrote:
quoted
On Tue 02-02-21 15:26:20, David Hildenbrand wrote:
quoted
On 02.02.21 15:22, Michal Hocko wrote:
quoted
On Tue 02-02-21 15:12:21, David Hildenbrand wrote:
[...]
quoted
I think secretmem behaves much more like longterm GUP right now
("unmigratable", "lifetime controlled by user space", "cannot go on
CMA/ZONE_MOVABLE"). I'd either want to reasonably well control/limit it or
make it behave more like mlocked pages.
I thought I have already asked but I must have forgotten. Is there any
actual reason why the memory is not movable? Timing attacks?
I think the reason is simple: no direct map, no copying of memory.
This is an implementation detail though and not something terribly hard
to add on top later on. I was more worried there would be really
fundamental reason why this is not possible. E.g. security implications.
I don't remember all the details. Let's see what Mike thinks regarding
migration (e.g., security concerns).
Thanks for considering me a security expert :-)
Yet, I cannot estimate how dangerous is the temporal exposure of
this data to the kernel via the direct map in the simple map/copy/unmap
sequence.
More secure way would be to map source and destination in a different page table
rather than in the direct map, similarly to the way text_poke() on x86
does.
I've left the migration callback empty for now because it can be added on
top and its implementation would depend on the way we do (or do not do)
pooling.
--
Sincerely yours,
Mike.
From: Mike Rapoport <rppt@kernel.org> Date: 2021-02-02 19:14:19
On Tue, Feb 02, 2021 at 02:27:14PM +0100, Michal Hocko wrote:
On Tue 02-02-21 14:48:57, Mike Rapoport wrote:
quoted
On Tue, Feb 02, 2021 at 10:35:05AM +0100, Michal Hocko wrote:
quoted
On Mon 01-02-21 08:56:19, James Bottomley wrote:
I have also proposed potential ways out of this. Either the pool is not
fixed sized and you make it a regular unevictable memory (if direct map
fragmentation is not considered a major problem)
I think that the direct map fragmentation is not a major problem, and the
data we have confirms it, so I'd be more than happy to entirely drop the
pool, allocate memory page by page and remove each page from the direct
map.
Still, we cannot prove negative and it could happen that there is a
workload that would suffer a lot from the direct map fragmentation, so
having a pool of large pages upfront is better than trying to fix it
afterwards. As we get more confidence that the direct map fragmentation is
not an issue as it is common to believe we may remove the pool altogether.
I would drop the pool altogether and instantiate pages to the
unevictable LRU list and internally treat it as ramdisk/mlock so you
will get an accounting correctly. The feature should be still opt-in
(e.g. a kernel command line parameter) for now. The recent report by
Intel (http://lkml.kernel.org/r/213b4567-46ce-f116-9cdf-bbd0c884eb3c@linux.intel.com)
there is no clear win to have huge mappings in _general_ but there are
still workloads which benefit.
quoted
I think that using PMD_ORDER allocations for the pool with a fallback to
order 0 will do the job, but unfortunately I doubt we'll reach a consensus
about this because dogmatic beliefs are hard to shake...
If this is opt-in then those beliefs can be relaxed somehow. Long term
it makes a lot of sense to optimize for a better direct map management
but I do not think this is a hard requirement for an initial
implementation if it is not imposed to everybody by default.
quoted
A more restrictive possibility is to still use plain PMD_ORDER allocations
to fill the pool, without relying on CMA. In this case there will be no
global secretmem specific pool to exhaust, but then it's possible to drain
high order free blocks in a system, so CMA has an advantage of limiting
secretmem pools to certain amount of memory with somewhat higher
probability for high order allocation to succeed.
quoted
or you need a careful access control
Do you mind elaborating what do you mean by "careful access control"?
As already mentioned, a mechanism to control who can use this feature -
e.g. make it a special device which you can access control by
permissions or higher level security policies. But that is really needed
only if the pool is fixed sized.
Let me reiterate to make sure I don't misread your suggestion.
If we make secretmem an opt-in feature with, e.g. kernel parameter, the
pooling of large pages is unnecessary. In this case there is no limited
resource we need to protect because secretmem will allocate page by page.
Since there is no limited resource, we don't need special permissions
to access secretmem so we can move forward with a system call that creates
a mmapable file descriptor and save the hassle of a chardev.
I cannot say I don't like this as it cuts roughly half of mm/secretmem.c :)
But I must say I am still a bit concerned about that we have no provisions
here for dealing with the direct map fragmentation even with the set goal
to improve the direct map management in the long run...
--
Sincerely yours,
Mike.
From: James Bottomley <hidden> Date: 2021-02-02 19:20:30
On Tue, 2021-02-02 at 20:15 +0200, Mike Rapoport wrote:
On Tue, Feb 02, 2021 at 03:34:29PM +0100, David Hildenbrand wrote:
quoted
On 02.02.21 15:32, Michal Hocko wrote:
quoted
On Tue 02-02-21 15:26:20, David Hildenbrand wrote:
quoted
On 02.02.21 15:22, Michal Hocko wrote:
quoted
On Tue 02-02-21 15:12:21, David Hildenbrand wrote:
[...]
quoted
I think secretmem behaves much more like longterm GUP right
now
("unmigratable", "lifetime controlled by user space",
"cannot go on
CMA/ZONE_MOVABLE"). I'd either want to reasonably well
control/limit it or
make it behave more like mlocked pages.
I thought I have already asked but I must have forgotten. Is
there any
actual reason why the memory is not movable? Timing attacks?
I think the reason is simple: no direct map, no copying of
memory.
This is an implementation detail though and not something
terribly hard
to add on top later on. I was more worried there would be really
fundamental reason why this is not possible. E.g. security
implications.
I don't remember all the details. Let's see what Mike thinks
regarding
migration (e.g., security concerns).
Thanks for considering me a security expert :-)
Yet, I cannot estimate how dangerous is the temporal exposure of
this data to the kernel via the direct map in the simple
map/copy/unmap
sequence.
Well the safest security statement is that we never expose the data to
the kernel because it's a very clean security statement and easy to
enforce. It's also the easiest threat model to analyse. Once we do
start exposing the secret to the kernel it alters the threat profile
and the analysis and obviously potentially provides the ROP gadget to
an attacker to do the same. Instinct tells me that the loss of
security doesn't really make up for the ability to swap or migrate but
if there were a case for doing the latter, it would have to be a
security policy of the user (i.e. a user should be able to decide their
data is too sensitive to expose to the kernel).
More secure way would be to map source and destination in a different
page table rather than in the direct map, similarly to the way
text_poke() on x86 does.
I think doing this would have much less of an impact on the security
posture because it's already theoretically possible to have kmap
restore access to the kernel.
James
I've left the migration callback empty for now because it can be
added on top and its implementation would depend on the way we do (or
do not do) pooling.
From: Michal Hocko <mhocko@suse.com> Date: 2021-02-03 09:14:06
On Tue 02-02-21 21:10:40, Mike Rapoport wrote:
On Tue, Feb 02, 2021 at 02:27:14PM +0100, Michal Hocko wrote:
quoted
On Tue 02-02-21 14:48:57, Mike Rapoport wrote:
quoted
On Tue, Feb 02, 2021 at 10:35:05AM +0100, Michal Hocko wrote:
quoted
On Mon 01-02-21 08:56:19, James Bottomley wrote:
I have also proposed potential ways out of this. Either the pool is not
fixed sized and you make it a regular unevictable memory (if direct map
fragmentation is not considered a major problem)
I think that the direct map fragmentation is not a major problem, and the
data we have confirms it, so I'd be more than happy to entirely drop the
pool, allocate memory page by page and remove each page from the direct
map.
Still, we cannot prove negative and it could happen that there is a
workload that would suffer a lot from the direct map fragmentation, so
having a pool of large pages upfront is better than trying to fix it
afterwards. As we get more confidence that the direct map fragmentation is
not an issue as it is common to believe we may remove the pool altogether.
I would drop the pool altogether and instantiate pages to the
unevictable LRU list and internally treat it as ramdisk/mlock so you
will get an accounting correctly. The feature should be still opt-in
(e.g. a kernel command line parameter) for now. The recent report by
Intel (http://lkml.kernel.org/r/213b4567-46ce-f116-9cdf-bbd0c884eb3c@linux.intel.com)
there is no clear win to have huge mappings in _general_ but there are
still workloads which benefit.
quoted
I think that using PMD_ORDER allocations for the pool with a fallback to
order 0 will do the job, but unfortunately I doubt we'll reach a consensus
about this because dogmatic beliefs are hard to shake...
If this is opt-in then those beliefs can be relaxed somehow. Long term
it makes a lot of sense to optimize for a better direct map management
but I do not think this is a hard requirement for an initial
implementation if it is not imposed to everybody by default.
quoted
A more restrictive possibility is to still use plain PMD_ORDER allocations
to fill the pool, without relying on CMA. In this case there will be no
global secretmem specific pool to exhaust, but then it's possible to drain
high order free blocks in a system, so CMA has an advantage of limiting
secretmem pools to certain amount of memory with somewhat higher
probability for high order allocation to succeed.
quoted
or you need a careful access control
Do you mind elaborating what do you mean by "careful access control"?
As already mentioned, a mechanism to control who can use this feature -
e.g. make it a special device which you can access control by
permissions or higher level security policies. But that is really needed
only if the pool is fixed sized.
Let me reiterate to make sure I don't misread your suggestion.
If we make secretmem an opt-in feature with, e.g. kernel parameter, the
pooling of large pages is unnecessary. In this case there is no limited
resource we need to protect because secretmem will allocate page by page.
Yes.
Since there is no limited resource, we don't need special permissions
to access secretmem so we can move forward with a system call that creates
a mmapable file descriptor and save the hassle of a chardev.
Yes, I assume you implicitly assume mlock rlimit here. Also memcg
accounting should be in place. Wrt to the specific syscall, please
document why existing interfaces are not a good fit as well. It would be
also great to describe interaction with mlock itself (I assume the two
to be incompatible - mlock will fail on and mlockall will ignore it).
I cannot say I don't like this as it cuts roughly half of mm/secretmem.c :)
But I must say I am still a bit concerned about that we have no provisions
here for dealing with the direct map fragmentation even with the set goal
to improve the direct map management in the long run...
Yes that is something that will be needed long term. I do not think this
is strictly necessary for the initial submission, though. The
implementation should be as simple as possible now and complexity added
on top.
--
Michal Hocko
SUSE Labs
From: Michal Hocko <mhocko@suse.com> Date: 2021-02-03 12:10:38
On Tue 02-02-21 10:55:40, James Bottomley wrote:
On Tue, 2021-02-02 at 20:15 +0200, Mike Rapoport wrote:
quoted
On Tue, Feb 02, 2021 at 03:34:29PM +0100, David Hildenbrand wrote:
quoted
On 02.02.21 15:32, Michal Hocko wrote:
quoted
On Tue 02-02-21 15:26:20, David Hildenbrand wrote:
quoted
On 02.02.21 15:22, Michal Hocko wrote:
quoted
On Tue 02-02-21 15:12:21, David Hildenbrand wrote:
[...]
quoted
I think secretmem behaves much more like longterm GUP right
now
("unmigratable", "lifetime controlled by user space",
"cannot go on
CMA/ZONE_MOVABLE"). I'd either want to reasonably well
control/limit it or
make it behave more like mlocked pages.
I thought I have already asked but I must have forgotten. Is
there any
actual reason why the memory is not movable? Timing attacks?
I think the reason is simple: no direct map, no copying of
memory.
This is an implementation detail though and not something
terribly hard
to add on top later on. I was more worried there would be really
fundamental reason why this is not possible. E.g. security
implications.
I don't remember all the details. Let's see what Mike thinks
regarding
migration (e.g., security concerns).
Thanks for considering me a security expert :-)
Yet, I cannot estimate how dangerous is the temporal exposure of
this data to the kernel via the direct map in the simple
map/copy/unmap
sequence.
Well the safest security statement is that we never expose the data to
the kernel because it's a very clean security statement and easy to
enforce. It's also the easiest threat model to analyse. Once we do
start exposing the secret to the kernel it alters the threat profile
and the analysis and obviously potentially provides the ROP gadget to
an attacker to do the same. Instinct tells me that the loss of
security doesn't really make up for the ability to swap or migrate but
if there were a case for doing the latter, it would have to be a
security policy of the user (i.e. a user should be able to decide their
data is too sensitive to expose to the kernel).
The security/threat model should be documented in the changelog as
well. I am not a security expert but I would tend to agree that not
allowing even temporal mapping for data copying (in the kernel) is the
most robust approach. Whether that is generally necessary for users I do
not know.
From the API POV I think it makes sense to have two
modes. NEVER_MAP_IN_KERNEL which would imply no migrateability, no
copy_{from,to}_user, no gup or any other way for the kernel to access
content of the memory. Maybe even zero the content on the last unmap to
never allow any data leak. ALLOW_TEMPORARY would unmap the page from
the direct mapping but it would still allow temporary mappings for
data copying inside the kernel (thus allow CoW, copy*user, migration).
Which one should be default and which an opt-in I do not know. A less
restrictive mode to be default and the more restrictive an opt-in via
flags makes a lot of sense to me though.
--
Michal Hocko
SUSE Labs
From: Mike Rapoport <rppt@kernel.org> Date: 2021-02-04 10:00:14
On Wed, Feb 03, 2021 at 10:12:22AM +0100, Michal Hocko wrote:
On Tue 02-02-21 21:10:40, Mike Rapoport wrote:
quoted
Let me reiterate to make sure I don't misread your suggestion.
If we make secretmem an opt-in feature with, e.g. kernel parameter, the
pooling of large pages is unnecessary. In this case there is no limited
resource we need to protect because secretmem will allocate page by page.
Yes.
quoted
Since there is no limited resource, we don't need special permissions
to access secretmem so we can move forward with a system call that creates
a mmapable file descriptor and save the hassle of a chardev.
Yes, I assume you implicitly assume mlock rlimit here.
Yes.
Also memcg accounting should be in place.
Right, without pools memcg accounting is no different from other
unevictable files.
Wrt to the specific syscall, please document why existing interfaces are
not a good fit as well. It would be also great to describe interaction
with mlock itself (I assume the two to be incompatible - mlock will fail
on and mlockall will ignore it).
The interaction with mlock() belongs more to the man page, but I don't mind
adding this to changelog as well.
--
Sincerely yours,
Mike.
From: Mike Rapoport <rppt@kernel.org> Date: 2021-02-04 11:35:16
On Wed, Feb 03, 2021 at 01:09:30PM +0100, Michal Hocko wrote:
On Tue 02-02-21 10:55:40, James Bottomley wrote:
quoted
On Tue, 2021-02-02 at 20:15 +0200, Mike Rapoport wrote:
quoted
On Tue, Feb 02, 2021 at 03:34:29PM +0100, David Hildenbrand wrote:
quoted
On 02.02.21 15:32, Michal Hocko wrote:
Well the safest security statement is that we never expose the data to
the kernel because it's a very clean security statement and easy to
enforce. It's also the easiest threat model to analyse. Once we do
start exposing the secret to the kernel it alters the threat profile
and the analysis and obviously potentially provides the ROP gadget to
an attacker to do the same. Instinct tells me that the loss of
security doesn't really make up for the ability to swap or migrate but
if there were a case for doing the latter, it would have to be a
security policy of the user (i.e. a user should be able to decide their
data is too sensitive to expose to the kernel).
The security/threat model should be documented in the changelog as
well. I am not a security expert but I would tend to agree that not
allowing even temporal mapping for data copying (in the kernel) is the
most robust approach. Whether that is generally necessary for users I do
not know.
From the API POV I think it makes sense to have two
modes. NEVER_MAP_IN_KERNEL which would imply no migrateability, no
copy_{from,to}_user, no gup or any other way for the kernel to access
content of the memory. Maybe even zero the content on the last unmap to
never allow any data leak. ALLOW_TEMPORARY would unmap the page from
the direct mapping but it would still allow temporary mappings for
data copying inside the kernel (thus allow CoW, copy*user, migration).
Which one should be default and which an opt-in I do not know. A less
restrictive mode to be default and the more restrictive an opt-in via
flags makes a lot of sense to me though.
The default is already NEVER_MAP_IN_KERNEL, so there is no explicit flag
for this. ALLOW_TEMPORARY should be opt-in, IMHO, and we can add it on top
later on.
--
Sincerely yours,
Mike.
From: Michal Hocko <mhocko@suse.com> Date: 2021-02-04 13:03:38
On Thu 04-02-21 11:58:55, Mike Rapoport wrote:
On Wed, Feb 03, 2021 at 10:12:22AM +0100, Michal Hocko wrote:
[...]
quoted
Wrt to the specific syscall, please document why existing interfaces are
not a good fit as well. It would be also great to describe interaction
with mlock itself (I assume the two to be incompatible - mlock will fail
on and mlockall will ignore it).
The interaction with mlock() belongs more to the man page, but I don't mind
adding this to changelog as well.
I would expect this to be explicitly handled in the patch - thus the
changelog rationale.
--
Michal Hocko
SUSE Labs