| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <edbd53c6-3396-9293-4a04-2b22655eeb12@gmail.com>
Date: Tue, 17 Apr 2018 12:22:48 +0300
From: Oleksandr Andrushchenko <andr2000@...il.com>
To: Juergen Gross <jgross@...e.com>, xen-devel@...ts.xenproject.org,
linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
boris.ostrovsky@...cle.com, konrad.wilk@...cle.com, perex@...ex.cz,
tiwai@...e.com
Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
Subject: Re: [PATCH v2 4/5] ALSA: xen-front: Implement handling of shared
buffers
On 04/16/2018 04:39 PM, Juergen Gross wrote:
> On 16/04/18 08:24, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
>>
>> Implement shared buffer handling according to the
>> para-virtualized sound device protocol at xen/interface/io/sndif.h:
>> - manage buffer memory
>> - handle granted references
>> - handle page directories
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
>> ---
>> sound/xen/Makefile | 3 +-
>> sound/xen/xen_snd_front.c | 8 ++
>> sound/xen/xen_snd_front_shbuf.c | 193 ++++++++++++++++++++++++++++++++++++++++
>> sound/xen/xen_snd_front_shbuf.h | 36 ++++++++
>> 4 files changed, 239 insertions(+), 1 deletion(-)
>> create mode 100644 sound/xen/xen_snd_front_shbuf.c
>> create mode 100644 sound/xen/xen_snd_front_shbuf.h
>>
>> diff --git a/sound/xen/Makefile b/sound/xen/Makefile
>> index 03c669984000..f028bc30af5d 100644
>> --- a/sound/xen/Makefile
>> +++ b/sound/xen/Makefile
>> @@ -2,6 +2,7 @@
>>
>> snd_xen_front-objs := xen_snd_front.o \
>> xen_snd_front_cfg.o \
>> - xen_snd_front_evtchnl.o
>> + xen_snd_front_evtchnl.o \
>> + xen_snd_front_shbuf.o
>>
>> obj-$(CONFIG_SND_XEN_FRONTEND) += snd_xen_front.o
>> diff --git a/sound/xen/xen_snd_front.c b/sound/xen/xen_snd_front.c
>> index eb46bf4070f9..0569c6c596a3 100644
>> --- a/sound/xen/xen_snd_front.c
>> +++ b/sound/xen/xen_snd_front.c
>> @@ -11,6 +11,7 @@
>> #include <linux/delay.h>
>> #include <linux/module.h>
>>
>> +#include <xen/page.h>
>> #include <xen/platform_pci.h>
>> #include <xen/xen.h>
>> #include <xen/xenbus.h>
>> @@ -186,6 +187,13 @@ static struct xenbus_driver xen_driver = {
>>
>> static int __init xen_drv_init(void)
>> {
>> + /* At the moment we only support case with XEN_PAGE_SIZE == PAGE_SIZE */
>> + if (XEN_PAGE_SIZE != PAGE_SIZE) {
>> + pr_err(XENSND_DRIVER_NAME ": different kernel and Xen page sizes are not supported: XEN_PAGE_SIZE (%lu) != PAGE_SIZE (%lu)\n",
>> + XEN_PAGE_SIZE, PAGE_SIZE);
>> + return -ENODEV;
>> + }
> Do you really want to print that error message on bare metal?
will move it down after xen_domain/xen_has_pv_devices checks
>> +
>> if (!xen_domain())
>> return -ENODEV;
>>
>> diff --git a/sound/xen/xen_snd_front_shbuf.c b/sound/xen/xen_snd_front_shbuf.c
>> new file mode 100644
>> index 000000000000..6845dbc7fdf5
>> --- /dev/null
>> +++ b/sound/xen/xen_snd_front_shbuf.c
>> @@ -0,0 +1,193 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>> +
>> +/*
>> + * Xen para-virtual sound device
>> + *
>> + * Copyright (C) 2016-2018 EPAM Systems Inc.
>> + *
>> + * Author: Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
>> + */
>> +
>> +#include <xen/xen.h>
>> +#include <xen/xenbus.h>
>> +
>> +#include "xen_snd_front_shbuf.h"
>> +
>> +grant_ref_t xen_snd_front_shbuf_get_dir_start(struct xen_snd_front_shbuf *buf)
>> +{
>> + if (!buf->grefs)
>> + return GRANT_INVALID_REF;
>> +
>> + return buf->grefs[0];
>> +}
>> +
>> +void xen_snd_front_shbuf_clear(struct xen_snd_front_shbuf *buf)
>> +{
>> + memset(buf, 0, sizeof(*buf));
>> +}
>> +
>> +void xen_snd_front_shbuf_free(struct xen_snd_front_shbuf *buf)
>> +{
>> + int i;
>> +
>> + if (buf->grefs) {
>> + for (i = 0; i < buf->num_grefs; i++)
>> + if (buf->grefs[i] != GRANT_INVALID_REF)
>> + gnttab_end_foreign_access(buf->grefs[i],
>> + 0, 0UL);
>> + kfree(buf->grefs);
>> + }
>> + kfree(buf->directory);
>> + free_pages_exact(buf->buffer, buf->buffer_sz);
>> + xen_snd_front_shbuf_clear(buf);
>> +}
>> +
>> +/*
>> + * number of grant references a page can hold with respect to the
>> + * xensnd_page_directory header
>> + */
>> +#define XENSND_NUM_GREFS_PER_PAGE ((XEN_PAGE_SIZE - \
>> + offsetof(struct xensnd_page_directory, gref)) / \
>> + sizeof(grant_ref_t))
>> +
>> +static void fill_page_dir(struct xen_snd_front_shbuf *buf,
>> + int num_pages_dir)
>> +{
>> + struct xensnd_page_directory *page_dir;
>> + unsigned char *ptr;
>> + int i, cur_gref, grefs_left, to_copy;
>> +
>> + ptr = buf->directory;
>> + grefs_left = buf->num_grefs - num_pages_dir;
>> + /*
>> + * skip grant references at the beginning, they are for pages granted
>> + * for the page directory itself
>> + */
>> + cur_gref = num_pages_dir;
>> + for (i = 0; i < num_pages_dir; i++) {
>> + page_dir = (struct xensnd_page_directory *)ptr;
>> + if (grefs_left <= XENSND_NUM_GREFS_PER_PAGE) {
>> + to_copy = grefs_left;
>> + page_dir->gref_dir_next_page = GRANT_INVALID_REF;
>> + } else {
>> + to_copy = XENSND_NUM_GREFS_PER_PAGE;
>> + page_dir->gref_dir_next_page = buf->grefs[i + 1];
>> + }
>> +
>> + memcpy(&page_dir->gref, &buf->grefs[cur_gref],
>> + to_copy * sizeof(grant_ref_t));
>> +
>> + ptr += XEN_PAGE_SIZE;
>> + grefs_left -= to_copy;
>> + cur_gref += to_copy;
>> + }
>> +}
>> +
>> +static int grant_references(struct xenbus_device *xb_dev,
>> + struct xen_snd_front_shbuf *buf,
>> + int num_pages_dir, int num_pages_buffer,
>> + int num_grefs)
>> +{
>> + grant_ref_t priv_gref_head;
>> + unsigned long frame;
>> + int ret, i, j, cur_ref;
>> + int otherend_id;
>> +
>> + ret = gnttab_alloc_grant_references(num_grefs, &priv_gref_head);
>> + if (ret)
>> + return ret;
>> +
>> + buf->num_grefs = num_grefs;
>> + otherend_id = xb_dev->otherend_id;
>> + j = 0;
>> +
>> + for (i = 0; i < num_pages_dir; i++) {
>> + cur_ref = gnttab_claim_grant_reference(&priv_gref_head);
>> + if (cur_ref < 0) {
>> + ret = cur_ref;
>> + goto fail;
>> + }
>> +
>> + frame = xen_page_to_gfn(virt_to_page(buf->directory +
>> + XEN_PAGE_SIZE * i));
>> + gnttab_grant_foreign_access_ref(cur_ref, otherend_id, frame, 0);
>> + buf->grefs[j++] = cur_ref;
>> + }
>> +
>> + for (i = 0; i < num_pages_buffer; i++) {
>> + cur_ref = gnttab_claim_grant_reference(&priv_gref_head);
>> + if (cur_ref < 0) {
>> + ret = cur_ref;
>> + goto fail;
>> + }
>> +
>> + frame = xen_page_to_gfn(virt_to_page(buf->buffer +
>> + XEN_PAGE_SIZE * i));
>> + gnttab_grant_foreign_access_ref(cur_ref, otherend_id, frame, 0);
>> + buf->grefs[j++] = cur_ref;
>> + }
>> +
>> + gnttab_free_grant_references(priv_gref_head);
>> + fill_page_dir(buf, num_pages_dir);
>> + return 0;
>> +
>> +fail:
>> + gnttab_free_grant_references(priv_gref_head);
>> + return ret;
>> +}
>> +
>> +static int alloc_int_buffers(struct xen_snd_front_shbuf *buf,
>> + int num_pages_dir, int num_pages_buffer,
>> + int num_grefs)
>> +{
>> + buf->grefs = kcalloc(num_grefs, sizeof(*buf->grefs), GFP_KERNEL);
>> + if (!buf->grefs)
>> + return -ENOMEM;
>> +
>> + buf->directory = kcalloc(num_pages_dir, XEN_PAGE_SIZE, GFP_KERNEL);
>> + if (!buf->directory)
>> + goto fail;
>> +
>> + buf->buffer_sz = num_pages_buffer * XEN_PAGE_SIZE;
>> + buf->buffer = alloc_pages_exact(buf->buffer_sz, GFP_KERNEL);
>> + if (!buf->buffer)
>> + goto fail;
>> +
>> + return 0;
>> +
>> +fail:
>> + kfree(buf->grefs);
>> + buf->grefs = NULL;
>> + kfree(buf->directory);
> Why do you need to free those here? Shouldn't that be done via
> xen_snd_front_shbuf_free() in case of an error?
At this place we only allocate memory, but xen_snd_front_shbuf_free
will also try to gnttab_end_foreign_access if buf->grefs != NULL.
> Juergen
Powered by blists - more mailing lists