linux-kernel - Re: [PATCH 2/3] xen/granttable: Grant tables V2 implementation

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 18 Nov 2011 11:02:45 +0000
From:	Ian Campbell <Ian.Campbell@...rix.com>
To:	ANNIE LI <annie.li@...cle.com>
CC:	"xen-devel@...ts.xensource.com" <xen-devel@...ts.xensource.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"konrad.wilk@...cle.com" <konrad.wilk@...cle.com>,
	"jeremy@...p.org" <jeremy@...p.org>,
	Paul Durrant <Paul.Durrant@...rix.com>,
	"kurt.hackel@...cle.com" <kurt.hackel@...cle.com>
Subject: Re: [PATCH 2/3] xen/granttable: Grant tables V2 implementation

On Fri, 2011-11-18 at 10:13 +0000, ANNIE LI wrote:
> static void gnttab_request_version(void)
>  {
> -       grant_table_version = 1;
> -       gnttab_interface = &gnttab_v1_ops;
> +       int rc;
> +       struct gnttab_set_version gsv;
> +       const char *str = "we need grant tables version 2, but only version 1 is available\n";
> +
> +       gsv.version = 2;
> +       rc = HYPERVISOR_grant_table_op(GNTTABOP_set_version, &gsv, 1);
> +       if (rc == 0) {
> +               grant_table_version = 2;
> +               gnttab_interface = &gnttab_v2_ops;
> +       } else {
> +               if (grant_table_version == 2) {

	  } else if (...) {
		panic
	  } else {
		setup v1
	  }

> +                       /*
> +                        * If we've already used version 2 features,
> +                        * but then suddenly discover that they're not
> +                        * available (e.g. migrating to an older
> +                        * version of Xen), almost unbounded badness
> +                        * can happen.
> +                        */
> +                       xen_raw_printk(str);
> +                       panic(str);

I expect you've just copied this style from elsewhere but I really
dislike this duplication of prints. If panic is not useful here we
really ought to address that at the root instead of going around
patching things to print every panic message twice. I thought
earlyprintk was supposed to solve this problem. Perhaps a generic
early_panic_print could be added to the panic code?

If we stick with this (which is fair enough since it is outside the
scope of this series) you should move the const char *str into this code
block so it is near to the use.

> +               }
> +               grant_table_version = 1;
> +               gnttab_interface = &gnttab_v1_ops;
> +       }
>         printk(KERN_INFO "Grant tables using version %d layout.\n",
>                 grant_table_version);
>  } 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/