Skip to content

Commit 35e884f

Browse files
committed
Merge tag 'for-linus-5.8b-rc4-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip
Pull xen fixes from Juergen Gross: "One small cleanup patch for ARM and two patches for the xenbus driver fixing latent problems (large stack allocations and bad return code settings)" * tag 'for-linus-5.8b-rc4-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip: xen/xenbus: let xenbus_map_ring_valloc() return errno values only xen/xenbus: avoid large structs and arrays on the stack arm/xen: remove the unused macro GRANT_TABLE_PHYSADDR
2 parents 8b082a4 + 578c1bb commit 35e884f

File tree

2 files changed

+81
-87
lines changed

2 files changed

+81
-87
lines changed

arch/arm/xen/enlighten.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,6 @@ static int __init fdt_find_hyper_node(unsigned long node, const char *uname,
241241
* see Documentation/devicetree/bindings/arm/xen.txt for the
242242
* documentation of the Xen Device Tree format.
243243
*/
244-
#define GRANT_TABLE_PHYSADDR 0
245244
void __init xen_early_init(void)
246245
{
247246
of_scan_flat_dt(fdt_find_hyper_node, NULL);

drivers/xen/xenbus/xenbus_client.c

Lines changed: 81 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,27 @@ struct xenbus_map_node {
6969
unsigned int nr_handles;
7070
};
7171

72+
struct map_ring_valloc {
73+
struct xenbus_map_node *node;
74+
75+
/* Why do we need two arrays? See comment of __xenbus_map_ring */
76+
union {
77+
unsigned long addrs[XENBUS_MAX_RING_GRANTS];
78+
pte_t *ptes[XENBUS_MAX_RING_GRANTS];
79+
};
80+
phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS];
81+
82+
struct gnttab_map_grant_ref map[XENBUS_MAX_RING_GRANTS];
83+
struct gnttab_unmap_grant_ref unmap[XENBUS_MAX_RING_GRANTS];
84+
85+
unsigned int idx; /* HVM only. */
86+
};
87+
7288
static DEFINE_SPINLOCK(xenbus_valloc_lock);
7389
static LIST_HEAD(xenbus_valloc_pages);
7490

7591
struct xenbus_ring_ops {
76-
int (*map)(struct xenbus_device *dev,
92+
int (*map)(struct xenbus_device *dev, struct map_ring_valloc *info,
7793
grant_ref_t *gnt_refs, unsigned int nr_grefs,
7894
void **vaddr);
7995
int (*unmap)(struct xenbus_device *dev, void *vaddr);
@@ -440,21 +456,33 @@ EXPORT_SYMBOL_GPL(xenbus_free_evtchn);
440456
* Map @nr_grefs pages of memory into this domain from another
441457
* domain's grant table. xenbus_map_ring_valloc allocates @nr_grefs
442458
* pages of virtual address space, maps the pages to that address, and
443-
* sets *vaddr to that address. Returns 0 on success, and GNTST_*
444-
* (see xen/include/interface/grant_table.h) or -ENOMEM / -EINVAL on
459+
* sets *vaddr to that address. Returns 0 on success, and -errno on
445460
* error. If an error is returned, device will switch to
446461
* XenbusStateClosing and the error message will be saved in XenStore.
447462
*/
448463
int xenbus_map_ring_valloc(struct xenbus_device *dev, grant_ref_t *gnt_refs,
449464
unsigned int nr_grefs, void **vaddr)
450465
{
451466
int err;
467+
struct map_ring_valloc *info;
468+
469+
*vaddr = NULL;
470+
471+
if (nr_grefs > XENBUS_MAX_RING_GRANTS)
472+
return -EINVAL;
473+
474+
info = kzalloc(sizeof(*info), GFP_KERNEL);
475+
if (!info)
476+
return -ENOMEM;
452477

453-
err = ring_ops->map(dev, gnt_refs, nr_grefs, vaddr);
454-
/* Some hypervisors are buggy and can return 1. */
455-
if (err > 0)
456-
err = GNTST_general_error;
478+
info->node = kzalloc(sizeof(*info->node), GFP_KERNEL);
479+
if (!info->node)
480+
err = -ENOMEM;
481+
else
482+
err = ring_ops->map(dev, info, gnt_refs, nr_grefs, vaddr);
457483

484+
kfree(info->node);
485+
kfree(info);
458486
return err;
459487
}
460488
EXPORT_SYMBOL_GPL(xenbus_map_ring_valloc);
@@ -466,62 +494,57 @@ static int __xenbus_map_ring(struct xenbus_device *dev,
466494
grant_ref_t *gnt_refs,
467495
unsigned int nr_grefs,
468496
grant_handle_t *handles,
469-
phys_addr_t *addrs,
497+
struct map_ring_valloc *info,
470498
unsigned int flags,
471499
bool *leaked)
472500
{
473-
struct gnttab_map_grant_ref map[XENBUS_MAX_RING_GRANTS];
474-
struct gnttab_unmap_grant_ref unmap[XENBUS_MAX_RING_GRANTS];
475501
int i, j;
476-
int err = GNTST_okay;
477502

478503
if (nr_grefs > XENBUS_MAX_RING_GRANTS)
479504
return -EINVAL;
480505

481506
for (i = 0; i < nr_grefs; i++) {
482-
memset(&map[i], 0, sizeof(map[i]));
483-
gnttab_set_map_op(&map[i], addrs[i], flags, gnt_refs[i],
484-
dev->otherend_id);
507+
gnttab_set_map_op(&info->map[i], info->phys_addrs[i], flags,
508+
gnt_refs[i], dev->otherend_id);
485509
handles[i] = INVALID_GRANT_HANDLE;
486510
}
487511

488-
gnttab_batch_map(map, i);
512+
gnttab_batch_map(info->map, i);
489513

490514
for (i = 0; i < nr_grefs; i++) {
491-
if (map[i].status != GNTST_okay) {
492-
err = map[i].status;
493-
xenbus_dev_fatal(dev, map[i].status,
515+
if (info->map[i].status != GNTST_okay) {
516+
xenbus_dev_fatal(dev, info->map[i].status,
494517
"mapping in shared page %d from domain %d",
495518
gnt_refs[i], dev->otherend_id);
496519
goto fail;
497520
} else
498-
handles[i] = map[i].handle;
521+
handles[i] = info->map[i].handle;
499522
}
500523

501-
return GNTST_okay;
524+
return 0;
502525

503526
fail:
504527
for (i = j = 0; i < nr_grefs; i++) {
505528
if (handles[i] != INVALID_GRANT_HANDLE) {
506-
memset(&unmap[j], 0, sizeof(unmap[j]));
507-
gnttab_set_unmap_op(&unmap[j], (phys_addr_t)addrs[i],
529+
gnttab_set_unmap_op(&info->unmap[j],
530+
info->phys_addrs[i],
508531
GNTMAP_host_map, handles[i]);
509532
j++;
510533
}
511534
}
512535

513-
if (HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, unmap, j))
536+
if (HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, info->unmap, j))
514537
BUG();
515538

516539
*leaked = false;
517540
for (i = 0; i < j; i++) {
518-
if (unmap[i].status != GNTST_okay) {
541+
if (info->unmap[i].status != GNTST_okay) {
519542
*leaked = true;
520543
break;
521544
}
522545
}
523546

524-
return err;
547+
return -ENOENT;
525548
}
526549

527550
/**
@@ -566,21 +589,12 @@ static int xenbus_unmap_ring(struct xenbus_device *dev, grant_handle_t *handles,
566589
return err;
567590
}
568591

569-
struct map_ring_valloc_hvm
570-
{
571-
unsigned int idx;
572-
573-
/* Why do we need two arrays? See comment of __xenbus_map_ring */
574-
phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS];
575-
unsigned long addrs[XENBUS_MAX_RING_GRANTS];
576-
};
577-
578592
static void xenbus_map_ring_setup_grant_hvm(unsigned long gfn,
579593
unsigned int goffset,
580594
unsigned int len,
581595
void *data)
582596
{
583-
struct map_ring_valloc_hvm *info = data;
597+
struct map_ring_valloc *info = data;
584598
unsigned long vaddr = (unsigned long)gfn_to_virt(gfn);
585599

586600
info->phys_addrs[info->idx] = vaddr;
@@ -589,39 +603,28 @@ static void xenbus_map_ring_setup_grant_hvm(unsigned long gfn,
589603
info->idx++;
590604
}
591605

592-
static int xenbus_map_ring_valloc_hvm(struct xenbus_device *dev,
593-
grant_ref_t *gnt_ref,
594-
unsigned int nr_grefs,
595-
void **vaddr)
606+
static int xenbus_map_ring_hvm(struct xenbus_device *dev,
607+
struct map_ring_valloc *info,
608+
grant_ref_t *gnt_ref,
609+
unsigned int nr_grefs,
610+
void **vaddr)
596611
{
597-
struct xenbus_map_node *node;
612+
struct xenbus_map_node *node = info->node;
598613
int err;
599614
void *addr;
600615
bool leaked = false;
601-
struct map_ring_valloc_hvm info = {
602-
.idx = 0,
603-
};
604616
unsigned int nr_pages = XENBUS_PAGES(nr_grefs);
605617

606-
if (nr_grefs > XENBUS_MAX_RING_GRANTS)
607-
return -EINVAL;
608-
609-
*vaddr = NULL;
610-
611-
node = kzalloc(sizeof(*node), GFP_KERNEL);
612-
if (!node)
613-
return -ENOMEM;
614-
615618
err = alloc_xenballooned_pages(nr_pages, node->hvm.pages);
616619
if (err)
617620
goto out_err;
618621

619622
gnttab_foreach_grant(node->hvm.pages, nr_grefs,
620623
xenbus_map_ring_setup_grant_hvm,
621-
&info);
624+
info);
622625

623626
err = __xenbus_map_ring(dev, gnt_ref, nr_grefs, node->handles,
624-
info.phys_addrs, GNTMAP_host_map, &leaked);
627+
info, GNTMAP_host_map, &leaked);
625628
node->nr_handles = nr_grefs;
626629

627630
if (err)
@@ -641,19 +644,20 @@ static int xenbus_map_ring_valloc_hvm(struct xenbus_device *dev,
641644
spin_unlock(&xenbus_valloc_lock);
642645

643646
*vaddr = addr;
647+
info->node = NULL;
648+
644649
return 0;
645650

646651
out_xenbus_unmap_ring:
647652
if (!leaked)
648-
xenbus_unmap_ring(dev, node->handles, nr_grefs, info.addrs);
653+
xenbus_unmap_ring(dev, node->handles, nr_grefs, info->addrs);
649654
else
650655
pr_alert("leaking %p size %u page(s)",
651656
addr, nr_pages);
652657
out_free_ballooned_pages:
653658
if (!leaked)
654659
free_xenballooned_pages(nr_pages, node->hvm.pages);
655660
out_err:
656-
kfree(node);
657661
return err;
658662
}
659663

@@ -676,40 +680,30 @@ int xenbus_unmap_ring_vfree(struct xenbus_device *dev, void *vaddr)
676680
EXPORT_SYMBOL_GPL(xenbus_unmap_ring_vfree);
677681

678682
#ifdef CONFIG_XEN_PV
679-
static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev,
680-
grant_ref_t *gnt_refs,
681-
unsigned int nr_grefs,
682-
void **vaddr)
683+
static int xenbus_map_ring_pv(struct xenbus_device *dev,
684+
struct map_ring_valloc *info,
685+
grant_ref_t *gnt_refs,
686+
unsigned int nr_grefs,
687+
void **vaddr)
683688
{
684-
struct xenbus_map_node *node;
689+
struct xenbus_map_node *node = info->node;
685690
struct vm_struct *area;
686-
pte_t *ptes[XENBUS_MAX_RING_GRANTS];
687-
phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS];
688691
int err = GNTST_okay;
689692
int i;
690693
bool leaked;
691694

692-
*vaddr = NULL;
693-
694-
if (nr_grefs > XENBUS_MAX_RING_GRANTS)
695-
return -EINVAL;
696-
697-
node = kzalloc(sizeof(*node), GFP_KERNEL);
698-
if (!node)
699-
return -ENOMEM;
700-
701-
area = alloc_vm_area(XEN_PAGE_SIZE * nr_grefs, ptes);
695+
area = alloc_vm_area(XEN_PAGE_SIZE * nr_grefs, info->ptes);
702696
if (!area) {
703697
kfree(node);
704698
return -ENOMEM;
705699
}
706700

707701
for (i = 0; i < nr_grefs; i++)
708-
phys_addrs[i] = arbitrary_virt_to_machine(ptes[i]).maddr;
702+
info->phys_addrs[i] =
703+
arbitrary_virt_to_machine(info->ptes[i]).maddr;
709704

710705
err = __xenbus_map_ring(dev, gnt_refs, nr_grefs, node->handles,
711-
phys_addrs,
712-
GNTMAP_host_map | GNTMAP_contains_pte,
706+
info, GNTMAP_host_map | GNTMAP_contains_pte,
713707
&leaked);
714708
if (err)
715709
goto failed;
@@ -722,6 +716,8 @@ static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev,
722716
spin_unlock(&xenbus_valloc_lock);
723717

724718
*vaddr = area->addr;
719+
info->node = NULL;
720+
725721
return 0;
726722

727723
failed:
@@ -730,11 +726,10 @@ static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev,
730726
else
731727
pr_alert("leaking VM area %p size %u page(s)", area, nr_grefs);
732728

733-
kfree(node);
734729
return err;
735730
}
736731

737-
static int xenbus_unmap_ring_vfree_pv(struct xenbus_device *dev, void *vaddr)
732+
static int xenbus_unmap_ring_pv(struct xenbus_device *dev, void *vaddr)
738733
{
739734
struct xenbus_map_node *node;
740735
struct gnttab_unmap_grant_ref unmap[XENBUS_MAX_RING_GRANTS];
@@ -798,12 +793,12 @@ static int xenbus_unmap_ring_vfree_pv(struct xenbus_device *dev, void *vaddr)
798793
}
799794

800795
static const struct xenbus_ring_ops ring_ops_pv = {
801-
.map = xenbus_map_ring_valloc_pv,
802-
.unmap = xenbus_unmap_ring_vfree_pv,
796+
.map = xenbus_map_ring_pv,
797+
.unmap = xenbus_unmap_ring_pv,
803798
};
804799
#endif
805800

806-
struct unmap_ring_vfree_hvm
801+
struct unmap_ring_hvm
807802
{
808803
unsigned int idx;
809804
unsigned long addrs[XENBUS_MAX_RING_GRANTS];
@@ -814,19 +809,19 @@ static void xenbus_unmap_ring_setup_grant_hvm(unsigned long gfn,
814809
unsigned int len,
815810
void *data)
816811
{
817-
struct unmap_ring_vfree_hvm *info = data;
812+
struct unmap_ring_hvm *info = data;
818813

819814
info->addrs[info->idx] = (unsigned long)gfn_to_virt(gfn);
820815

821816
info->idx++;
822817
}
823818

824-
static int xenbus_unmap_ring_vfree_hvm(struct xenbus_device *dev, void *vaddr)
819+
static int xenbus_unmap_ring_hvm(struct xenbus_device *dev, void *vaddr)
825820
{
826821
int rv;
827822
struct xenbus_map_node *node;
828823
void *addr;
829-
struct unmap_ring_vfree_hvm info = {
824+
struct unmap_ring_hvm info = {
830825
.idx = 0,
831826
};
832827
unsigned int nr_pages;
@@ -887,8 +882,8 @@ enum xenbus_state xenbus_read_driver_state(const char *path)
887882
EXPORT_SYMBOL_GPL(xenbus_read_driver_state);
888883

889884
static const struct xenbus_ring_ops ring_ops_hvm = {
890-
.map = xenbus_map_ring_valloc_hvm,
891-
.unmap = xenbus_unmap_ring_vfree_hvm,
885+
.map = xenbus_map_ring_hvm,
886+
.unmap = xenbus_unmap_ring_hvm,
892887
};
893888

894889
void __init xenbus_ring_ops_init(void)

0 commit comments

Comments
 (0)