Limits for ION Memory Allocator

Limits for ION Memory Allocator

[Alexander Popov]

Hello!

The syzkaller [1] has a trouble with fuzzing the Linux kernel with ION Memory
Allocator.

Syzkaller uses several methods [2] to limit memory consumption of the userspace
processes calling the syscalls for testing the kernel:
 - setrlimit(),
 - cgroups,
 - various sysctl.
But these methods don't work for ION Memory Allocator, so any userspace process
that has access to /dev/ion can bring the system to the out-of-memory state.

An example of a program doing that:


#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdio.h>
#include <linux/types.h>
#include <sys/ioctl.h>

#define ION_IOC_MAGIC		'I'
#define ION_IOC_ALLOC		_IOWR(ION_IOC_MAGIC, 0, \
				      struct ion_allocation_data)

struct ion_allocation_data {
	__u64 len;
	__u32 heap_id_mask;
	__u32 flags;
	__u32 fd;
	__u32 unused;
};

int main(void)
{
	unsigned long i = 0;
	int fd = -1;
	struct ion_allocation_data data = {
		.len = 0x13f65d8c,
		.heap_id_mask = 1,
		.flags = 0,
		.fd = -1,
		.unused = 0
	};

	fd = open("/dev/ion", 0);
	if (fd == -1) {
		perror("[-] open /dev/ion");
		return 1;
	}

	while (1) {
		printf("iter %lu\n", i);
		ioctl(fd, ION_IOC_ALLOC, &data);
		i++;
	}

	return 0;
}


I looked through the code of ion_alloc() and didn't find any limit checks.
Is it currently possible to limit ION kernel allocations for some process?

If not, is it a right idea to do that?
Thanks!

Best regards,
Alexander


[1]: https://github.com/google/syzkaller
[2]: https://github.com/google/syzkaller/blob/master/executor/common_linux.h

Re: Limits for ION Memory Allocator

[Laura Abbott]

On 7/17/19 12:31 PM, Alexander Popov wrote:
> Hello!
> 
> The syzkaller [1] has a trouble with fuzzing the Linux kernel with ION Memory
> Allocator.
> 
> Syzkaller uses several methods [2] to limit memory consumption of the userspace
> processes calling the syscalls for testing the kernel:
>   - setrlimit(),
>   - cgroups,
>   - various sysctl.
> But these methods don't work for ION Memory Allocator, so any userspace process
> that has access to /dev/ion can bring the system to the out-of-memory state.
> 
> An example of a program doing that:
> 
> 
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <stdio.h>
> #include <linux/types.h>
> #include <sys/ioctl.h>
> 
> #define ION_IOC_MAGIC		'I'
> #define ION_IOC_ALLOC		_IOWR(ION_IOC_MAGIC, 0, \
> 				      struct ion_allocation_data)
> 
> struct ion_allocation_data {
> 	__u64 len;
> 	__u32 heap_id_mask;
> 	__u32 flags;
> 	__u32 fd;
> 	__u32 unused;
> };
> 
> int main(void)
> {
> 	unsigned long i = 0;
> 	int fd = -1;
> 	struct ion_allocation_data data = {
> 		.len = 0x13f65d8c,
> 		.heap_id_mask = 1,
> 		.flags = 0,
> 		.fd = -1,
> 		.unused = 0
> 	};
> 
> 	fd = open("/dev/ion", 0);
> 	if (fd == -1) {
> 		perror("[-] open /dev/ion");
> 		return 1;
> 	}
> 
> 	while (1) {
> 		printf("iter %lu\n", i);
> 		ioctl(fd, ION_IOC_ALLOC, &data);
> 		i++;
> 	}
> 
> 	return 0;
> }
> 
> 
> I looked through the code of ion_alloc() and didn't find any limit checks.
> Is it currently possible to limit ION kernel allocations for some process?
> 
> If not, is it a right idea to do that?
> Thanks!
> 

Yes, I do think that's the right approach. We're working on moving Ion
out of staging and this is something I mentioned to John Stultz. I don't
think we've thought too hard about how to do the actual limiting so
suggestions are welcome.

Thanks,
Laura

> Best regards,
> Alexander
> 
> 
> [1]: https://github.com/google/syzkaller
> [2]: https://github.com/google/syzkaller/blob/master/executor/common_linux.h
> 

Re: Limits for ION Memory Allocator

[John Stultz]

On Wed, Jul 24, 2019 at 12:36 PM Laura Abbott <labbott@redhat.com> wrote:
>
> On 7/17/19 12:31 PM, Alexander Popov wrote:
> > Hello!
> >
> > The syzkaller [1] has a trouble with fuzzing the Linux kernel with ION Memory
> > Allocator.
> >
> > Syzkaller uses several methods [2] to limit memory consumption of the userspace
> > processes calling the syscalls for testing the kernel:
> >   - setrlimit(),
> >   - cgroups,
> >   - various sysctl.
> > But these methods don't work for ION Memory Allocator, so any userspace process
> > that has access to /dev/ion can bring the system to the out-of-memory state.
> >
> > An example of a program doing that:
> >
> >
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > #include <fcntl.h>
> > #include <stdio.h>
> > #include <linux/types.h>
> > #include <sys/ioctl.h>
> >
> > #define ION_IOC_MAGIC         'I'
> > #define ION_IOC_ALLOC         _IOWR(ION_IOC_MAGIC, 0, \
> >                                     struct ion_allocation_data)
> >
> > struct ion_allocation_data {
> >       __u64 len;
> >       __u32 heap_id_mask;
> >       __u32 flags;
> >       __u32 fd;
> >       __u32 unused;
> > };
> >
> > int main(void)
> > {
> >       unsigned long i = 0;
> >       int fd = -1;
> >       struct ion_allocation_data data = {
> >               .len = 0x13f65d8c,
> >               .heap_id_mask = 1,
> >               .flags = 0,
> >               .fd = -1,
> >               .unused = 0
> >       };
> >
> >       fd = open("/dev/ion", 0);
> >       if (fd == -1) {
> >               perror("[-] open /dev/ion");
> >               return 1;
> >       }
> >
> >       while (1) {
> >               printf("iter %lu\n", i);
> >               ioctl(fd, ION_IOC_ALLOC, &data);
> >               i++;
> >       }
> >
> >       return 0;
> > }
> >
> >
> > I looked through the code of ion_alloc() and didn't find any limit checks.
> > Is it currently possible to limit ION kernel allocations for some process?
> >
> > If not, is it a right idea to do that?
> > Thanks!
> >
>
> Yes, I do think that's the right approach. We're working on moving Ion
> out of staging and this is something I mentioned to John Stultz. I don't
> think we've thought too hard about how to do the actual limiting so
> suggestions are welcome.

In part the dmabuf heaps allow for separate heap devices, so we can
have finer grained permissions to the specific heaps.  But that
doesn't provide any controls on how much memory one process could
allocate using the device if it has permission.

I suspect the same issue is present with any of the dmabuf exporters
(gpu/display drivers, etc), so this is less of an ION/dmabuf heap
issue and more of a dmabuf core accounting issue.

Another practical complication is that with Android these days, I
believe the gralloc code lives in the HIDL-ized
android.hardware.graphics.allocator@2.0-service HAL, which does the
buffer allocations on behalf of requests sent over the binder IPC
interface. So with all dma-buf allocations effectively going through
that single process, I'm not sure we would want to put per-process
limits on the allocator.  Instead, I suspect we'd want the memory
covered by the dmabuf to be accounted against processes that have the
dmabuf fd still open?

I know Android has some logic with their memtrack HAL to I believe try
to do accounting of gpu memory against various processes, but I've not
looked at that in detail recently.

Todd/Joel: Any input here?

thanks
-john

Re: Limits for ION Memory Allocator

[John Stultz]

On Wed, Jul 24, 2019 at 1:18 PM John Stultz <john.stultz@linaro.org> wrote:
>
> On Wed, Jul 24, 2019 at 12:36 PM Laura Abbott <labbott@redhat.com> wrote:
> >
> > On 7/17/19 12:31 PM, Alexander Popov wrote:
> > > Hello!
> > >
> > > The syzkaller [1] has a trouble with fuzzing the Linux kernel with ION Memory
> > > Allocator.
> > >
> > > Syzkaller uses several methods [2] to limit memory consumption of the userspace
> > > processes calling the syscalls for testing the kernel:
> > >   - setrlimit(),
> > >   - cgroups,
> > >   - various sysctl.
> > > But these methods don't work for ION Memory Allocator, so any userspace process
> > > that has access to /dev/ion can bring the system to the out-of-memory state.
> > >
> > > An example of a program doing that:
> > >
> > >
> > > #include <sys/types.h>
> > > #include <sys/stat.h>
> > > #include <fcntl.h>
> > > #include <stdio.h>
> > > #include <linux/types.h>
> > > #include <sys/ioctl.h>
> > >
> > > #define ION_IOC_MAGIC         'I'
> > > #define ION_IOC_ALLOC         _IOWR(ION_IOC_MAGIC, 0, \
> > >                                     struct ion_allocation_data)
> > >
> > > struct ion_allocation_data {
> > >       __u64 len;
> > >       __u32 heap_id_mask;
> > >       __u32 flags;
> > >       __u32 fd;
> > >       __u32 unused;
> > > };
> > >
> > > int main(void)
> > > {
> > >       unsigned long i = 0;
> > >       int fd = -1;
> > >       struct ion_allocation_data data = {
> > >               .len = 0x13f65d8c,
> > >               .heap_id_mask = 1,
> > >               .flags = 0,
> > >               .fd = -1,
> > >               .unused = 0
> > >       };
> > >
> > >       fd = open("/dev/ion", 0);
> > >       if (fd == -1) {
> > >               perror("[-] open /dev/ion");
> > >               return 1;
> > >       }
> > >
> > >       while (1) {
> > >               printf("iter %lu\n", i);
> > >               ioctl(fd, ION_IOC_ALLOC, &data);
> > >               i++;
> > >       }
> > >
> > >       return 0;
> > > }
> > >
> > >
> > > I looked through the code of ion_alloc() and didn't find any limit checks.
> > > Is it currently possible to limit ION kernel allocations for some process?
> > >
> > > If not, is it a right idea to do that?
> > > Thanks!
> > >
> >
> > Yes, I do think that's the right approach. We're working on moving Ion
> > out of staging and this is something I mentioned to John Stultz. I don't
> > think we've thought too hard about how to do the actual limiting so
> > suggestions are welcome.
>
> In part the dmabuf heaps allow for separate heap devices, so we can
> have finer grained permissions to the specific heaps.  But that
> doesn't provide any controls on how much memory one process could
> allocate using the device if it has permission.
>
> I suspect the same issue is present with any of the dmabuf exporters
> (gpu/display drivers, etc), so this is less of an ION/dmabuf heap
> issue and more of a dmabuf core accounting issue.
>

Also, do unmapped memfd buffers have similar accounting issues?

thanks
-john

Re: Limits for ION Memory Allocator

[Joel Fernandes]

On Wed, Jul 24, 2019 at 4:24 PM John Stultz <john.stultz@linaro.org> wrote:
>
> On Wed, Jul 24, 2019 at 1:18 PM John Stultz <john.stultz@linaro.org> wrote:
> >
> > On Wed, Jul 24, 2019 at 12:36 PM Laura Abbott <labbott@redhat.com> wrote:
> > >
> > > On 7/17/19 12:31 PM, Alexander Popov wrote:
> > > > Hello!
> > > >
> > > > The syzkaller [1] has a trouble with fuzzing the Linux kernel with ION Memory
> > > > Allocator.
> > > >
> > > > Syzkaller uses several methods [2] to limit memory consumption of the userspace
> > > > processes calling the syscalls for testing the kernel:
> > > >   - setrlimit(),
> > > >   - cgroups,
> > > >   - various sysctl.
> > > > But these methods don't work for ION Memory Allocator, so any userspace process
> > > > that has access to /dev/ion can bring the system to the out-of-memory state.
> > > >
> > > > An example of a program doing that:
> > > >
> > > >
> > > > #include <sys/types.h>
> > > > #include <sys/stat.h>
> > > > #include <fcntl.h>
> > > > #include <stdio.h>
> > > > #include <linux/types.h>
> > > > #include <sys/ioctl.h>
> > > >
> > > > #define ION_IOC_MAGIC         'I'
> > > > #define ION_IOC_ALLOC         _IOWR(ION_IOC_MAGIC, 0, \
> > > >                                     struct ion_allocation_data)
> > > >
> > > > struct ion_allocation_data {
> > > >       __u64 len;
> > > >       __u32 heap_id_mask;
> > > >       __u32 flags;
> > > >       __u32 fd;
> > > >       __u32 unused;
> > > > };
> > > >
> > > > int main(void)
> > > > {
> > > >       unsigned long i = 0;
> > > >       int fd = -1;
> > > >       struct ion_allocation_data data = {
> > > >               .len = 0x13f65d8c,
> > > >               .heap_id_mask = 1,
> > > >               .flags = 0,
> > > >               .fd = -1,
> > > >               .unused = 0
> > > >       };
> > > >
> > > >       fd = open("/dev/ion", 0);
> > > >       if (fd == -1) {
> > > >               perror("[-] open /dev/ion");
> > > >               return 1;
> > > >       }
> > > >
> > > >       while (1) {
> > > >               printf("iter %lu\n", i);
> > > >               ioctl(fd, ION_IOC_ALLOC, &data);
> > > >               i++;
> > > >       }
> > > >
> > > >       return 0;
> > > > }
> > > >
> > > >
> > > > I looked through the code of ion_alloc() and didn't find any limit checks.
> > > > Is it currently possible to limit ION kernel allocations for some process?
> > > >
> > > > If not, is it a right idea to do that?
> > > > Thanks!
> > > >
> > >
> > > Yes, I do think that's the right approach. We're working on moving Ion
> > > out of staging and this is something I mentioned to John Stultz. I don't
> > > think we've thought too hard about how to do the actual limiting so
> > > suggestions are welcome.
> >
> > In part the dmabuf heaps allow for separate heap devices, so we can
> > have finer grained permissions to the specific heaps.  But that
> > doesn't provide any controls on how much memory one process could
> > allocate using the device if it has permission.
> >
> > I suspect the same issue is present with any of the dmabuf exporters
> > (gpu/display drivers, etc), so this is less of an ION/dmabuf heap
> > issue and more of a dmabuf core accounting issue.
> >
>
> Also, do unmapped memfd buffers have similar accounting issues?
>

The syzcaller bot didn't complain about this for memfd yet, so I suspect not ;-)

With memfd since it uses shmem underneath, __vm_enough_memory() is
called during shmem_acct_block() which should take per-process memory
into account already and fail if there is not enough memory.

Should ION be doing something similar to fail if there's not enough memory?

thanks,

- Joel