lkml.org 
[lkml]   [2020]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v3 06/12] ppc64/kexec_file: restrict memory usage of kdump kernel
Date

Hari Bathini <hbathini@linux.ibm.com> writes:

> /**
> + * get_usable_memory_ranges - Get usable memory ranges. This list includes
> + * regions like crashkernel, opal/rtas & tce-table,
> + * that kdump kernel could use.
> + * @mem_ranges: Range list to add the memory ranges to.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int get_usable_memory_ranges(struct crash_mem **mem_ranges)
> +{
> + int ret;
> +
> + /* First memory block & crashkernel region */
> + ret = add_mem_range(mem_ranges, 0, crashk_res.end + 1);

This is a bit surprising. I guess I don't have a complete big picture of
the patch series yet. What prevents the crashkernel from using memory at
the [0, _end] range and overwriting the crashed kernel's memory?

Shouldn't the above range start at crashk_res.start?

> + if (ret)
> + goto out;
> +
> + ret = add_rtas_mem_range(mem_ranges);
> + if (ret)
> + goto out;
> +
> + ret = add_opal_mem_range(mem_ranges);
> + if (ret)
> + goto out;
> +
> + ret = add_tce_mem_ranges(mem_ranges);
> +out:
> + if (ret)
> + pr_err("Failed to setup usable memory ranges\n");
> + return ret;
> +}
> +
> +/**
> * __locate_mem_hole_top_down - Looks top down for a large enough memory hole
> * in the memory regions between buf_min & buf_max
> * for the buffer. If found, sets kbuf->mem.
> @@ -261,6 +305,322 @@ static int locate_mem_hole_bottom_up_ppc64(struct kexec_buf *kbuf,
> }
>
> /**
> + * check_realloc_usable_mem - Reallocate buffer if it can't accommodate entries
> + * @um_info: Usable memory buffer and ranges info.
> + * @cnt: No. of entries to accommodate.
> + *
> + * Returns 0 on success, negative errno on error.

It actually returns the buffer on success, and NULL on error.

> + */
> +static uint64_t *check_realloc_usable_mem(struct umem_info *um_info, int cnt)
> +{
> + void *tbuf;
> +
> + if (um_info->size >=
> + ((um_info->idx + cnt) * sizeof(*(um_info->buf))))
> + return um_info->buf;
> +
> + um_info->size += MEM_RANGE_CHUNK_SZ;
> + tbuf = krealloc(um_info->buf, um_info->size, GFP_KERNEL);
> + if (!tbuf) {
> + um_info->size -= MEM_RANGE_CHUNK_SZ;
> + return NULL;
> + }
> +
> + memset(tbuf + um_info->idx, 0, MEM_RANGE_CHUNK_SZ);
> + return tbuf;
> +}

<snip>

> +/**
> + * get_node_path - Get the full path of the given node.
> + * @dn: Node.
> + * @path: Updated with the full path of the node.
> + *
> + * Returns nothing.
> + */
> +static void get_node_path(struct device_node *dn, char *path)
> +{
> + if (!dn)
> + return;
> +
> + get_node_path(dn->parent, path);

Is it ok to do recursion in the kernel? In this case I believe it's not
problematic since the maximum call depth will be the maximum depth of a
device tree node which shouldn't be too much. Also, there are no local
variables in this function. But I thought it was worth mentioning.

> + sprintf(path, "/%s", dn->full_name);
> +}
> +
> +/**
> + * get_node_pathlen - Get the full path length of the given node.
> + * @dn: Node.
> + *
> + * Returns the length of the full path of the node.
> + */
> +static int get_node_pathlen(struct device_node *dn)
> +{
> + int len = 0;
> +
> + while (dn) {
> + len += strlen(dn->full_name) + 1;
> + dn = dn->parent;
> + }
> + len++;
> +
> + return len;
> +}
> +
> +/**
> + * add_usable_mem_property - Add usable memory property for the given
> + * memory node.
> + * @fdt: Flattened device tree for the kdump kernel.
> + * @dn: Memory node.
> + * @um_info: Usable memory buffer and ranges info.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int add_usable_mem_property(void *fdt, struct device_node *dn,
> + struct umem_info *um_info)
> +{
> + int n_mem_addr_cells, n_mem_size_cells, node;
> + int i, len, ranges, cnt, ret;
> + uint64_t base, end, *buf;
> + const __be32 *prop;
> + char *pathname;
> +
> + /* Allocate memory for node path */
> + pathname = kzalloc(ALIGN(get_node_pathlen(dn), 8), GFP_KERNEL);
> + if (!pathname)
> + return -ENOMEM;
> +
> + /* Get the full path of the memory node */
> + get_node_path(dn, pathname);
> + pr_debug("Memory node path: %s\n", pathname);
> +
> + /* Now that we know the path, find its offset in kdump kernel's fdt */
> + node = fdt_path_offset(fdt, pathname);
> + if (node < 0) {
> + pr_err("Malformed device tree: error reading %s\n",
> + pathname);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + /* Get the address & size cells */
> + n_mem_addr_cells = of_n_addr_cells(dn);
> + n_mem_size_cells = of_n_size_cells(dn);
> + pr_debug("address cells: %d, size cells: %d\n", n_mem_addr_cells,
> + n_mem_size_cells);
> +
> + um_info->idx = 0;
> + buf = check_realloc_usable_mem(um_info, 2);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + um_info->buf = buf;
> +
> + prop = of_get_property(dn, "reg", &len);
> + if (!prop || len <= 0) {
> + ret = 0;
> + goto out;
> + }
> +
> + /*
> + * "reg" property represents sequence of (addr,size) duples

s/duples/tuples/ ?

> + * each representing a memory range.
> + */
> + ranges = (len >> 2) / (n_mem_addr_cells + n_mem_size_cells);
> +
> + for (i = 0; i < ranges; i++) {
> + base = of_read_number(prop, n_mem_addr_cells);
> + prop += n_mem_addr_cells;
> + end = base + of_read_number(prop, n_mem_size_cells) - 1;

You need to `prop += n_mem_size_cells` here.

> +
> + ret = add_usable_mem(um_info, base, end, &cnt);
> + if (ret) {
> + ret = ret;
> + goto out;
> + }
> + }
> +
> + /*
> + * No kdump kernel usable memory found in this memory node.
> + * Write (0,0) duple in linux,usable-memory property for

s/duple/tuple/ ?

> + * this region to be ignored.
> + */
> + if (um_info->idx == 0) {
> + um_info->buf[0] = 0;
> + um_info->buf[1] = 0;
> + um_info->idx = 2;
> + }
> +
> + ret = fdt_setprop(fdt, node, "linux,usable-memory", um_info->buf,
> + (um_info->idx * sizeof(*(um_info->buf))));
> +
> +out:
> + kfree(pathname);
> + return ret;
> +}

--
Thiago Jung Bauermann
IBM Linux Technology Center

\
 
 \ /
  Last update: 2020-07-16 00:53    [W:0.386 / U:0.960 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site