Re: acpithinkpad: a fix for the x260

2019-03-06 Thread Renato Aguiar

On Wed, Mar 06 2019, joshua stein wrote:

sthen found that the HKEY version metric failed on the x260 
where it 
reports version 1 but requires the new ACPI method of changing 
backlight.


This diff tries to do the ACPI method on all machines and falls 
back 
to the CMOS method if that fails.


Can all those that tried the previous diff (which has been 
committed) try this one and make sure nothing broke?


This also unmasks the microphone mute event which helps mixerctl 
stay in sync on the x260 (it has no effect on my x1c6, but 
probably 
can't hurt).



Index: sys/dev/acpi/acpithinkpad.c
===
RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
retrieving revision 1.63
diff -u -p -u -p -r1.63 acpithinkpad.c
--- sys/dev/acpi/acpithinkpad.c	6 Mar 2019 15:36:30 - 
1.63

+++ sys/dev/acpi/acpithinkpad.c 7 Mar 2019 02:53:36 -
@@ -124,6 +124,7 @@
  #define   THINKPAD_ADAPTIVE_MODE_HOME 1
  #define   THINKPAD_ADAPTIVE_MODE_FUNCTION 3
  
+#define THINKPAD_MASK_MIC_MUTE		(1 << 14)

  #define THINKPAD_MASK_BRIGHTNESS_UP   (1 << 15)
  #define THINKPAD_MASK_BRIGHTNESS_DOWN (1 << 16)
  #define THINKPAD_MASK_KBD_BACKLIGHT   (1 << 17)
@@ -171,8 +172,8 @@ int	thinkpad_get_kbd_backlight(struct 
ws

  int   thinkpad_set_kbd_backlight(struct wskbd_backlight *);
  extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
  extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
-void   thinkpad_get_brightness(struct acpithinkpad_softc *);
-void   thinkpad_set_brightness(void *, int);
+intthinkpad_get_brightness(struct acpithinkpad_softc *);
+intthinkpad_set_brightness(void *, int);
  int   thinkpad_get_param(struct wsdisplay_param *);
  int   thinkpad_set_param(struct wsdisplay_param *);
  extern int (*ws_get_param)(struct wsdisplay_param *);
@@ -345,7 +346,9 @@ thinkpad_enable_events(struct acpithinkp
}
  
  	/* Enable events we need to know about */
-	mask |= (THINKPAD_MASK_BRIGHTNESS_UP | 
THINKPAD_MASK_BRIGHTNESS_DOWN |

+   mask |= (THINKPAD_MASK_MIC_MUTE |
+   THINKPAD_MASK_BRIGHTNESS_UP |
+   THINKPAD_MASK_BRIGHTNESS_DOWN |
THINKPAD_MASK_KBD_BACKLIGHT);
  
  	DPRINTF(("%s: setting event mask to 0x%llx\n", 
  DEVNAME(sc), mask));

@@ -555,8 +558,7 @@ thinkpad_brightness_up(struct acpithinkp
  {
int b;
  
-	if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {

-   thinkpad_get_brightness(sc);
+   if (thinkpad_get_brightness(sc) == 0) {
b = sc->sc_brightness & 0xff;
if (b < ((sc->sc_brightness >> 8) & 0xff)) {
sc->sc_brightness = b + 1;
@@ -573,8 +575,7 @@ thinkpad_brightness_down(struct acpithin
  {
int b;
  
-	if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {

-   thinkpad_get_brightness(sc);
+   if (thinkpad_get_brightness(sc) == 0) {
b = sc->sc_brightness & 0xff;
if (b > 0) {
sc->sc_brightness = b - 1;
@@ -701,30 +702,39 @@ thinkpad_set_kbd_backlight(struct wskbd_
return 0;
  }
  
-void

+int
  thinkpad_get_brightness(struct acpithinkpad_softc *sc)
  {
-   aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
-   "PBLG", 0, NULL, >sc_brightness);
+   int ret;
+
+	ret = aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG", 
0, NULL,

+   >sc_brightness);
  
  	DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, 
  sc->sc_brightness));

+
+   return ret;
  }
  
-void

+int
  thinkpad_set_brightness(void *arg0, int arg1)
  {
struct acpithinkpad_softc *sc = arg0;
struct aml_value arg;
+   int ret;
  
  	DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, 
  sc->sc_brightness));
  
  	memset(, 0, sizeof(arg));

arg.type = AML_OBJTYPE_INTEGER;
arg.v_integer = sc->sc_brightness & 0xff;
-   aml_evalname(sc->sc_acpi, sc->sc_devnode,
-   "PBLS", 1, , NULL);
+	ret = aml_evalname(sc->sc_acpi, sc->sc_devnode, "PBLS", 1, 
, NULL);

+
+   if (ret)
+   return ret;
  
  	thinkpad_get_brightness(sc);

+
+   return 0;
  }
  
  int

@@ -765,7 +775,8 @@ thinkpad_set_param(struct wsdisplay_para
dp->curval = maxval;
sc->sc_brightness &= ~0xff;
sc->sc_brightness |= dp->curval;
-		acpi_addtask(sc->sc_acpi, thinkpad_set_brightness, 
sc, 0);
+		acpi_addtask(sc->sc_acpi, (void 
*)thinkpad_set_brightness, sc,

+   0);
acpi_wakeup(sc->sc_acpi);
return 0;
default:


Hi Joshua,

I didn't notice any regression with the new patch on x230 or 
t470p.


Also, microphone mute event seems to have no effect on t470p.

Regards,

--
Renato Aguiar



Re: Avoid system(3) in ikectl

2019-03-06 Thread Theo de Raadt
I'm not sure why this matters.

Fundamentally system is fork+exec via a shell.  So you write it as
minimal fork+exec.

What is the particular benefit you see here, is it security -- and if
so, what is the security benefit?  Have you identified a quoting problem?
Can you pinpoint the issue and explain it please?

> I had sent a similar patch a while back. There seemed to me some
> interest, but it was never comitted. Updated to apply to -current.
> 
> Apologies for the attachment; gmail still isn't sending emails sent
> via mutt, but I suspect the patch in the body will be mangled.
> 
> - Matthew Martin"
> 
> diff --git ikeca.c ikeca.c
> index bac76ab9c2f..01e600abb2c 100644
> --- ikeca.c
> +++ ikeca.c
> @@ -18,6 +18,7 @@
> 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -63,12 +64,12 @@
> 
>  struct ca {
>   char sslpath[PATH_MAX];
> - char passfile[PATH_MAX];
> + char passfile[PATH_MAX + 5];
>   char index[PATH_MAX];
>   char serial[PATH_MAX];
>   char sslcnf[PATH_MAX];
>   char extcnf[PATH_MAX];
> - char batch[PATH_MAX];
> + char *batch;
>   char *caname;
>  };
> 
> @@ -116,6 +117,7 @@ void ca_setenv(const char *, const char *);
>  void ca_clrenv(void);
>  void ca_setcnf(struct ca *, const char *);
>  void ca_create_index(struct ca *);
> +int static run(char *const []);
> 
>  /* util.c */
>  int expand_string(char *, size_t, const char *, const char *);
> @@ -130,7 +132,6 @@ int
>  ca_key_create(struct ca *ca, char *keyname)
>  {
>   struct stat st;
> - char cmd[PATH_MAX * 2];
>   char path[PATH_MAX];
> 
>   snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
> @@ -140,10 +141,8 @@ ca_key_create(struct ca *ca, char *keyname)
>   return (0);
>   }
> 
> - snprintf(cmd, sizeof(cmd),
> - "%s genrsa -out %s 2048",
> - PATH_OPENSSL, path);
> - system(cmd);
> + char *cmd[] = { PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL };
> + run(cmd);
>   chmod(path, 0600);
> 
>   return (0);
> @@ -200,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
>  int
>  ca_request(struct ca *ca, char *keyname, int type)
>  {
> - char cmd[PATH_MAX * 2];
>   char hostname[HOST_NAME_MAX+1];
>   char name[128];
> + char key[PATH_MAX];
>   char path[PATH_MAX];
> 
>   ca_setenv("$ENV::CERT_CN", keyname);
> @@ -226,13 +225,12 @@ ca_request(struct ca *ca, char *keyname, int type)
> 
>   ca_setcnf(ca, keyname);
> 
> + snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
>   snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
> - snprintf(cmd, sizeof(cmd), "%s req %s-new"
> - " -key %s/private/%s.key -out %s -config %s",
> - PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
> - path, ca->sslcnf);
> 
> - system(cmd);
> + char *cmd[] = { PATH_OPENSSL, "req", "-new", "-key", key, "-out", path,
> + "-config", ca->sslcnf, ca->batch, NULL };
> + run(cmd);
>   chmod(path, 0600);
> 
>   return (0);
> @@ -241,8 +239,11 @@ ca_request(struct ca *ca, char *keyname, int type)
>  int
>  ca_sign(struct ca *ca, char *keyname, int type)
>  {
> - char cmd[PATH_MAX * 2];
> - const char *extensions = NULL;
> + char cakey[PATH_MAX];
> + char cacrt[PATH_MAX];
> + char out[PATH_MAX];
> + char in[PATH_MAX];
> + char *extensions = NULL;
> 
>   if (type == HOST_IPADDR) {
>   extensions = "x509v3_IPAddr";
> @@ -258,19 +259,16 @@ ca_sign(struct ca *ca, char *keyname, int type)
>   ca_setenv("$ENV::CASERIAL", ca->serial);
>   ca_setcnf(ca, keyname);
> 
> - snprintf(cmd, sizeof(cmd),
> - "%s ca -config %s -keyfile %s/private/ca.key"
> - " -cert %s/ca.crt"
> - " -extfile %s -extensions %s -out %s/%s.crt"
> - " -in %s/private/%s.csr"
> - " -passin file:%s -outdir %s -batch",
> - PATH_OPENSSL, ca->sslcnf, ca->sslpath,
> - ca->sslpath,
> - ca->extcnf, extensions, ca->sslpath, keyname,
> - ca->sslpath, keyname,
> - ca->passfile, ca->sslpath);
> + snprintf(cakey, sizeof(cakey), "%s/private/ca.key", ca->sslpath);
> + snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
> + snprintf(out, sizeof(out), "%s/%s.crt", ca->sslpath, keyname);
> + snprintf(in, sizeof(in), "%s/private/%s.csr", ca->sslpath, keyname);
> 
> - system(cmd);
> + char *cmd[] = { PATH_OPENSSL, "ca", "-config", ca->sslcnf,
> + "-keyfile", cakey, "-cert", cacrt, "-extfile", ca->extcnf,
> + "-extensions", extensions, "-out", out, "-in", in,
> + "-passin", ca->passfile, "-outdir", ca->sslpath, "-batch", NULL };
> + run(cmd);
> 
>   return (0);
>  }
> @@ -313,9 +311,9 @@ int
>  ca_key_install(struct ca *ca, char *keyname, char *dir)
>  {
>   struct stat st;
> - char cmd[PATH_MAX * 2];
>   char src[PATH_MAX];
>   char dst[PATH_MAX];
> + char out[PATH_MAX];
>   char *p = NULL;
> 
>   snprintf(src, sizeof(src), "%s/private/%s.key", ca->sslpath, keyname);
> @@ -335,9 +333,11 @@ ca_key_install(struct ca *ca, char *keyname, char *dir)
>   snprintf(dst, sizeof(dst), "%s/private/local.key", dir);
>   fcopy(src, dst, 0600);
> 
> - snprintf(cmd, 

Avoid system(3) in ikectl

2019-03-06 Thread Matthew Martin
I had sent a similar patch a while back. There seemed to me some
interest, but it was never comitted. Updated to apply to -current.

Apologies for the attachment; gmail still isn't sending emails sent
via mutt, but I suspect the patch in the body will be mangled.

- Matthew Martin"

diff --git ikeca.c ikeca.c
index bac76ab9c2f..01e600abb2c 100644
--- ikeca.c
+++ ikeca.c
@@ -18,6 +18,7 @@

 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -63,12 +64,12 @@

 struct ca {
  char sslpath[PATH_MAX];
- char passfile[PATH_MAX];
+ char passfile[PATH_MAX + 5];
  char index[PATH_MAX];
  char serial[PATH_MAX];
  char sslcnf[PATH_MAX];
  char extcnf[PATH_MAX];
- char batch[PATH_MAX];
+ char *batch;
  char *caname;
 };

@@ -116,6 +117,7 @@ void ca_setenv(const char *, const char *);
 void ca_clrenv(void);
 void ca_setcnf(struct ca *, const char *);
 void ca_create_index(struct ca *);
+int static run(char *const []);

 /* util.c */
 int expand_string(char *, size_t, const char *, const char *);
@@ -130,7 +132,6 @@ int
 ca_key_create(struct ca *ca, char *keyname)
 {
  struct stat st;
- char cmd[PATH_MAX * 2];
  char path[PATH_MAX];

  snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
@@ -140,10 +141,8 @@ ca_key_create(struct ca *ca, char *keyname)
  return (0);
  }

- snprintf(cmd, sizeof(cmd),
- "%s genrsa -out %s 2048",
- PATH_OPENSSL, path);
- system(cmd);
+ char *cmd[] = { PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL };
+ run(cmd);
  chmod(path, 0600);

  return (0);
@@ -200,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
 int
 ca_request(struct ca *ca, char *keyname, int type)
 {
- char cmd[PATH_MAX * 2];
  char hostname[HOST_NAME_MAX+1];
  char name[128];
+ char key[PATH_MAX];
  char path[PATH_MAX];

  ca_setenv("$ENV::CERT_CN", keyname);
@@ -226,13 +225,12 @@ ca_request(struct ca *ca, char *keyname, int type)

  ca_setcnf(ca, keyname);

+ snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
  snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
- snprintf(cmd, sizeof(cmd), "%s req %s-new"
- " -key %s/private/%s.key -out %s -config %s",
- PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
- path, ca->sslcnf);

- system(cmd);
+ char *cmd[] = { PATH_OPENSSL, "req", "-new", "-key", key, "-out", path,
+ "-config", ca->sslcnf, ca->batch, NULL };
+ run(cmd);
  chmod(path, 0600);

  return (0);
@@ -241,8 +239,11 @@ ca_request(struct ca *ca, char *keyname, int type)
 int
 ca_sign(struct ca *ca, char *keyname, int type)
 {
- char cmd[PATH_MAX * 2];
- const char *extensions = NULL;
+ char cakey[PATH_MAX];
+ char cacrt[PATH_MAX];
+ char out[PATH_MAX];
+ char in[PATH_MAX];
+ char *extensions = NULL;

  if (type == HOST_IPADDR) {
  extensions = "x509v3_IPAddr";
@@ -258,19 +259,16 @@ ca_sign(struct ca *ca, char *keyname, int type)
  ca_setenv("$ENV::CASERIAL", ca->serial);
  ca_setcnf(ca, keyname);

- snprintf(cmd, sizeof(cmd),
- "%s ca -config %s -keyfile %s/private/ca.key"
- " -cert %s/ca.crt"
- " -extfile %s -extensions %s -out %s/%s.crt"
- " -in %s/private/%s.csr"
- " -passin file:%s -outdir %s -batch",
- PATH_OPENSSL, ca->sslcnf, ca->sslpath,
- ca->sslpath,
- ca->extcnf, extensions, ca->sslpath, keyname,
- ca->sslpath, keyname,
- ca->passfile, ca->sslpath);
+ snprintf(cakey, sizeof(cakey), "%s/private/ca.key", ca->sslpath);
+ snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
+ snprintf(out, sizeof(out), "%s/%s.crt", ca->sslpath, keyname);
+ snprintf(in, sizeof(in), "%s/private/%s.csr", ca->sslpath, keyname);

- system(cmd);
+ char *cmd[] = { PATH_OPENSSL, "ca", "-config", ca->sslcnf,
+ "-keyfile", cakey, "-cert", cacrt, "-extfile", ca->extcnf,
+ "-extensions", extensions, "-out", out, "-in", in,
+ "-passin", ca->passfile, "-outdir", ca->sslpath, "-batch", NULL };
+ run(cmd);

  return (0);
 }
@@ -313,9 +311,9 @@ int
 ca_key_install(struct ca *ca, char *keyname, char *dir)
 {
  struct stat st;
- char cmd[PATH_MAX * 2];
  char src[PATH_MAX];
  char dst[PATH_MAX];
+ char out[PATH_MAX];
  char *p = NULL;

  snprintf(src, sizeof(src), "%s/private/%s.key", ca->sslpath, keyname);
@@ -335,9 +333,11 @@ ca_key_install(struct ca *ca, char *keyname, char *dir)
  snprintf(dst, sizeof(dst), "%s/private/local.key", dir);
  fcopy(src, dst, 0600);

- snprintf(cmd, sizeof(cmd), "%s rsa -out %s/local.pub"
- " -in %s/private/local.key -pubout", PATH_OPENSSL, dir, dir);
- system(cmd);
+ snprintf(out, sizeof(out), "%s/local.pub", dir);
+
+ char *cmd[] = { PATH_OPENSSL, "rsa", "-out", out, "-in", dst,
+ "-pubout", NULL };
+ run(cmd);

  free(p);

@@ -405,37 +405,41 @@ ca_newpass(char *passfile, char *password)
 int
 ca_create(struct ca *ca)
 {
- char cmd[PATH_MAX * 2];
- char path[PATH_MAX];
+ char key[PATH_MAX];
+ char csr[PATH_MAX];
+ char crt[PATH_MAX];

  ca_clrenv();

- snprintf(path, sizeof(path), "%s/private/ca.key", ca->sslpath);
- snprintf(cmd, sizeof(cmd), "%s 

acpithinkpad: a fix for the x260

2019-03-06 Thread joshua stein
sthen found that the HKEY version metric failed on the x260 where it 
reports version 1 but requires the new ACPI method of changing 
backlight.

This diff tries to do the ACPI method on all machines and falls back 
to the CMOS method if that fails.

Can all those that tried the previous diff (which has been 
committed) try this one and make sure nothing broke?

This also unmasks the microphone mute event which helps mixerctl 
stay in sync on the x260 (it has no effect on my x1c6, but probably 
can't hurt).


Index: sys/dev/acpi/acpithinkpad.c
===
RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
retrieving revision 1.63
diff -u -p -u -p -r1.63 acpithinkpad.c
--- sys/dev/acpi/acpithinkpad.c 6 Mar 2019 15:36:30 -   1.63
+++ sys/dev/acpi/acpithinkpad.c 7 Mar 2019 02:53:36 -
@@ -124,6 +124,7 @@
  #define   THINKPAD_ADAPTIVE_MODE_HOME 1
  #define   THINKPAD_ADAPTIVE_MODE_FUNCTION 3
  
+#define THINKPAD_MASK_MIC_MUTE (1 << 14)
  #define THINKPAD_MASK_BRIGHTNESS_UP   (1 << 15)
  #define THINKPAD_MASK_BRIGHTNESS_DOWN (1 << 16)
  #define THINKPAD_MASK_KBD_BACKLIGHT   (1 << 17)
@@ -171,8 +172,8 @@ int thinkpad_get_kbd_backlight(struct ws
  int   thinkpad_set_kbd_backlight(struct wskbd_backlight *);
  extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
  extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
-void   thinkpad_get_brightness(struct acpithinkpad_softc *);
-void   thinkpad_set_brightness(void *, int);
+intthinkpad_get_brightness(struct acpithinkpad_softc *);
+intthinkpad_set_brightness(void *, int);
  int   thinkpad_get_param(struct wsdisplay_param *);
  int   thinkpad_set_param(struct wsdisplay_param *);
  extern int (*ws_get_param)(struct wsdisplay_param *);
@@ -345,7 +346,9 @@ thinkpad_enable_events(struct acpithinkp
}
  
/* Enable events we need to know about */
-   mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
+   mask |= (THINKPAD_MASK_MIC_MUTE |
+   THINKPAD_MASK_BRIGHTNESS_UP |
+   THINKPAD_MASK_BRIGHTNESS_DOWN |
THINKPAD_MASK_KBD_BACKLIGHT);
  
DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
@@ -555,8 +558,7 @@ thinkpad_brightness_up(struct acpithinkp
  {
int b;
  
-   if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
-   thinkpad_get_brightness(sc);
+   if (thinkpad_get_brightness(sc) == 0) {
b = sc->sc_brightness & 0xff;
if (b < ((sc->sc_brightness >> 8) & 0xff)) {
sc->sc_brightness = b + 1;
@@ -573,8 +575,7 @@ thinkpad_brightness_down(struct acpithin
  {
int b;
  
-   if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
-   thinkpad_get_brightness(sc);
+   if (thinkpad_get_brightness(sc) == 0) {
b = sc->sc_brightness & 0xff;
if (b > 0) {
sc->sc_brightness = b - 1;
@@ -701,30 +702,39 @@ thinkpad_set_kbd_backlight(struct wskbd_
return 0;
  }
  
-void
+int
  thinkpad_get_brightness(struct acpithinkpad_softc *sc)
  {
-   aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
-   "PBLG", 0, NULL, >sc_brightness);
+   int ret;
+
+   ret = aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG", 0, NULL,
+   >sc_brightness);
  
DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, sc->sc_brightness));
+
+   return ret;
  }
  
-void
+int
  thinkpad_set_brightness(void *arg0, int arg1)
  {
struct acpithinkpad_softc *sc = arg0;
struct aml_value arg;
+   int ret;
  
DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, sc->sc_brightness));
  
memset(, 0, sizeof(arg));
arg.type = AML_OBJTYPE_INTEGER;
arg.v_integer = sc->sc_brightness & 0xff;
-   aml_evalname(sc->sc_acpi, sc->sc_devnode,
-   "PBLS", 1, , NULL);
+   ret = aml_evalname(sc->sc_acpi, sc->sc_devnode, "PBLS", 1, , NULL);
+
+   if (ret)
+   return ret;
  
thinkpad_get_brightness(sc);
+
+   return 0;
  }
  
  int
@@ -765,7 +775,8 @@ thinkpad_set_param(struct wsdisplay_para
dp->curval = maxval;
sc->sc_brightness &= ~0xff;
sc->sc_brightness |= dp->curval;
-   acpi_addtask(sc->sc_acpi, thinkpad_set_brightness, sc, 0);
+   acpi_addtask(sc->sc_acpi, (void *)thinkpad_set_brightness, sc,
+   0);
acpi_wakeup(sc->sc_acpi);
return 0;
default:



Re: libcrypto: INTEGER_cmp vs. STRING_cmp

2019-03-06 Thread Holger Mikolon


> Date: Wed, 6 Mar 2019 06:31:17
> From: Theo Buehler 

(snip)

> If you're up for it, it would probably be a good idea to look at the
> changes introduced by the commit you mentioned and see what else looks
> suspicious and needs fixing.

(snip)

I went through the files affected by said commit and focused on INTEGER 
vs. STRING mixup only (mostly related to serialNumber, once related to 
zone). Then I greped through the rest of libcrypto sources and found just 
x_crl.c to have a mixup.

I did not touch asn1/a_strnid.c, where the serialNumber is listed as
B_ASN1_PRINTABLESTRING. I don't know enough here, so I better leave
this for the experts.

Holger  


Index: asn1/x_crl.c
===
RCS file: /cvs/src/lib/libcrypto/asn1/x_crl.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 x_crl.c
--- asn1/x_crl.c24 Aug 2018 19:55:58 -  1.33
+++ asn1/x_crl.c6 Mar 2019 21:46:52 -
@@ -527,9 +527,7 @@ X509_CRL_dup(X509_CRL *x)
 static int
 X509_REVOKED_cmp(const X509_REVOKED * const *a, const X509_REVOKED * const *b)
 {
-   return(ASN1_STRING_cmp(
-   (ASN1_STRING *)(*a)->serialNumber,
-   (ASN1_STRING *)(*b)->serialNumber));
+   return(ASN1_INTEGER_cmp((*a)->serialNumber, (*b)->serialNumber));
 }
 
 int
Index: pkcs7/pk7_doit.c
===
RCS file: /cvs/src/lib/libcrypto/pkcs7/pk7_doit.c,v
retrieving revision 1.42
diff -u -p -u -r1.42 pk7_doit.c
--- pkcs7/pk7_doit.c2 May 2017 03:59:45 -   1.42
+++ pkcs7/pk7_doit.c6 Mar 2019 21:46:52 -
@@ -410,7 +410,7 @@ pkcs7_cmp_ri(PKCS7_RECIP_INFO *ri, X509 
pcert->cert_info->issuer);
if (ret)
return ret;
-   return ASN1_STRING_cmp(pcert->cert_info->serialNumber,
+   return ASN1_INTEGER_cmp(pcert->cert_info->serialNumber,
ri->issuer_and_serial->serial);
 }
 
Index: pkcs7/pk7_lib.c
===
RCS file: /cvs/src/lib/libcrypto/pkcs7/pk7_lib.c,v
retrieving revision 1.19
diff -u -p -u -r1.19 pk7_lib.c
--- pkcs7/pk7_lib.c 29 Jan 2017 17:49:23 -  1.19
+++ pkcs7/pk7_lib.c 6 Mar 2019 21:46:53 -
@@ -374,7 +374,7 @@ PKCS7_SIGNER_INFO_set(PKCS7_SIGNER_INFO 
 * things the ugly way. */
ASN1_INTEGER_free(p7i->issuer_and_serial->serial);
if (!(p7i->issuer_and_serial->serial =
-   ASN1_STRING_dup(X509_get_serialNumber(x509
+   ASN1_INTEGER_dup(X509_get_serialNumber(x509
goto err;
 
/* lets keep the pkey around for a while */
@@ -534,7 +534,7 @@ PKCS7_RECIP_INFO_set(PKCS7_RECIP_INFO *p
 
ASN1_INTEGER_free(p7i->issuer_and_serial->serial);
if (!(p7i->issuer_and_serial->serial =
-   ASN1_STRING_dup(X509_get_serialNumber(x509
+   ASN1_INTEGER_dup(X509_get_serialNumber(x509
return 0;
 
pkey = X509_get_pubkey(x509);
Index: x509/x509_cmp.c
===
RCS file: /cvs/src/lib/libcrypto/x509/x509_cmp.c,v
retrieving revision 1.34
diff -u -p -u -r1.34 x509_cmp.c
--- x509/x509_cmp.c 24 Aug 2018 19:59:32 -  1.34
+++ x509/x509_cmp.c 6 Mar 2019 21:46:53 -
@@ -76,7 +76,7 @@ X509_issuer_and_serial_cmp(const X509 *a
 
ai = a->cert_info;
bi = b->cert_info;
-   i = ASN1_STRING_cmp(ai->serialNumber, bi->serialNumber);
+   i = ASN1_INTEGER_cmp(ai->serialNumber, bi->serialNumber);
if (i)
return (i);
return (X509_NAME_cmp(ai->issuer, bi->issuer));
Index: x509v3/v3_sxnet.c
===
RCS file: /cvs/src/lib/libcrypto/x509v3/v3_sxnet.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 v3_sxnet.c
--- x509v3/v3_sxnet.c   13 May 2018 15:03:01 -  1.21
+++ x509v3/v3_sxnet.c   6 Mar 2019 21:46:53 -
@@ -376,7 +376,7 @@ SXNET_get_id_INTEGER(SXNET *sx, ASN1_INT
 
for (i = 0; i < sk_SXNETID_num(sx->ids); i++) {
id = sk_SXNETID_value(sx->ids, i);
-   if (!ASN1_STRING_cmp(id->zone, zone))
+   if (!ASN1_INTEGER_cmp(id->zone, zone))
return id->user;
}
return NULL;



Re: acpithinkpad: fix brightness keys, keyboard backlight value

2019-03-06 Thread Juan Francisco Cantero Hurtado
On Tue, Mar 05, 2019 at 02:03:13PM -0600, joshua stein wrote:
> Here we go again...
> 
> On at least the ThinkPad X1C6, the screen brightness keys (F5 and 
> F6) do not work and "wsconsctl keyboard.backlight" doesn't report 
> the correct value when the keyboard backlight is adjusted with 
> Fn+Space.
> 
> These are both caused by the default event mask not including these 
> events, so explicitly enable them.
> 
> But then acpithinkpad has to actually do something for the screen 
> brightness keys, but it tries the very old CMOS method which doesn't 
> work on these newer machines[0].  So make it use the ACPI method.
> 
> I renamed thinkpad_[gs]et_backlight to thinkpad_[gs]et_kbd_backlight 
> because it was confusing that they have nothing to do with screen 
> backlight.
> 
> 
> 0. "newer machines" being those with MHKV reporting version 2.  If 
> this diff breaks on older "newer machines", this metric will have to 
> be changed to something else.

The brightness keys on the X61s still work fine.


-- 
Juan Francisco Cantero Hurtado http://juanfra.info



Re: relayd websocket

2019-03-06 Thread Rivo Nurges
Hi!


On 3/6/19 10:20 PM, Rivo Nurges wrote:
> On 3/6/19 6:36 PM, Sebastian Benoit wrote:
>>> Does something like this make sense?
>>
>> i think the seperator list needs to include '\t'
>> because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
>>
>> And i dont think you can mix "," with " \t" seperators,
>> because otherwise "Foo Upgrade, Bar" will match.
>>
>> Something more is needed to parse elements of a header.
> 
> Oh yeah. I'll work on that.

So here comes the next version. Works with both spaces and tabs.

Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.72
diff -u -p -r1.72 relay_http.c
--- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 -   1.72
+++ usr.sbin/relayd/relay_http.c6 Mar 2019 20:53:59 -
@@ -36,6 +36,7 @@
  #include 
  #include 
  #include 
+#include 

  #include "relayd.h"
  #include "http.h"
@@ -166,6 +167,7 @@ relay_read_http(struct bufferevent *bev,
struct relay_http_priv  *priv = con->se_priv;
char*line = NULL, *key, *value;
char*urlproto, *host, *path;
+   char*valuecopy, *valuepart;
int  action, unique, ret;
const char  *errstr;
size_t   size, linelen;
@@ -399,10 +401,19 @@ relay_read_http(struct bufferevent *bev,

if (cre->line != 1) {
if (cre->dir == RELAY_DIR_REQUEST) {
-   if (strcasecmp("Connection", key) == 0 &&
-   strcasecmp("Upgrade", value) == 0)
-   priv->http_upgrade_req |=
-   HTTP_CONNECTION_UPGRADE;
+   if (strcasecmp("Connection", key) == 0) {
+   valuecopy = strdup(value);
+   while ((valuepart = strsep(,
+   ",")) != NULL) {
+   while (isblank(*valuepart))
+   valuepart = [1];
+   if (strcasecmp("Upgrade", valuepart)
+   == 0)
+   priv->http_upgrade_req |=
+   HTTP_CONNECTION_UPGRADE;
+   }
+   free(valuecopy);
+   }
if (strcasecmp("Upgrade", key) == 0 &&
strcasecmp("websocket", value) == 0)
priv->http_upgrade_req |=


begin-base64 644 websocket3.diff
SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog
L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp
b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5
ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp
bi9yZWxheWQvcmVsYXlfaHR0cC5jCTYgTWFyIDIwMTkgMjA6NTM6NTkgLTAwMDAKQEAgLTM2LDYg
KzM2LDcgQEAKICNpbmNsdWRlIDxzaXBoYXNoLmg+CiAjaW5jbHVkZSA8aW1zZy5oPgogI2luY2x1
ZGUgPHVuaXN0ZC5oPgorI2luY2x1ZGUgPGN0eXBlLmg+CiAKICNpbmNsdWRlICJyZWxheWQuaCIK
ICNpbmNsdWRlICJodHRwLmgiCkBAIC0xNjYsNiArMTY3LDcgQEAgcmVsYXlfcmVhZF9odHRwKHN0
cnVjdCBidWZmZXJldmVudCAqYmV2LAogCXN0cnVjdCByZWxheV9odHRwX3ByaXYJKnByaXYgPSBj
b24tPnNlX3ByaXY7CiAJY2hhcgkJCSpsaW5lID0gTlVMTCwgKmtleSwgKnZhbHVlOwogCWNoYXIJ
CQkqdXJscHJvdG8sICpob3N0LCAqcGF0aDsKKwljaGFyCQkJKnZhbHVlY29weSwgKnZhbHVlcGFy
dDsKIAlpbnQJCQkgYWN0aW9uLCB1bmlxdWUsIHJldDsKIAljb25zdCBjaGFyCQkqZXJyc3RyOwog
CXNpemVfdAkJCSBzaXplLCBsaW5lbGVuOwpAQCAtMzk5LDEwICs0MDEsMTkgQEAgcmVsYXlfcmVh
ZF9odHRwKHN0cnVjdCBidWZmZXJldmVudCAqYmV2LAogCiAJCWlmIChjcmUtPmxpbmUgIT0gMSkg
ewogCQkJaWYgKGNyZS0+ZGlyID09IFJFTEFZX0RJUl9SRVFVRVNUKSB7Ci0JCQkJaWYgKHN0cmNh
c2VjbXAoIkNvbm5lY3Rpb24iLCBrZXkpID09IDAgJiYKLQkJCQkgICAgc3RyY2FzZWNtcCgiVXBn
cmFkZSIsIHZhbHVlKSA9PSAwKQotCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Ci0JCQkJ
CSAgICBIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVj
dGlvbiIsIGtleSkgPT0gMCkgeworCQkJCSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOwor
CQkJCSAgICB3aGlsZSAoKHZhbHVlcGFydCA9IHN0cnNlcCgmdmFsdWVjb3B5LAorCQkJCQkiLCIp
KSAhPSBOVUxMKSB7CisJCQkJCXdoaWxlIChpc2JsYW5rKCp2YWx1ZXBhcnQpKQorCQkJCQkgICAg
dmFsdWVwYXJ0ID0gJnZhbHVlcGFydFsxXTsKKwkJCQkgICAgCWlmIChzdHJjYXNlY21wKCJVcGdy
YWRlIiwgdmFsdWVwYXJ0KQorCQkJCQkgICAgPT0gMCkKKwkJCQkJICAgIHByaXYtPmh0dHBfdXBn
cmFkZV9yZXEgfD0KKwkJCQkJICAgIAlIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQkgICAg
fQorCQkJCSAgICBmcmVlKHZhbHVlY29weSk7CisJCQkJfQogCQkJCWlmIChzdHJjYXNlY21wKCJV
cGdyYWRlIiwga2V5KSA9PSAwICYmCiAJCQkJICAgIHN0cmNhc2VjbXAoIndlYnNvY2tldCIsIHZh

Re: relayd websocket

2019-03-06 Thread Rivo Nurges
On 3/6/19 6:36 PM, Sebastian Benoit wrote:
>> Does something like this make sense?
> 
> i think the seperator list needs to include '\t'
> because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
> 
> And i dont think you can mix "," with " \t" seperators,
> because otherwise "Foo Upgrade, Bar" will match.
> 
> Something more is needed to parse elements of a header.

Oh yeah. I'll work on that.

Rivo



Re: relayd websocket

2019-03-06 Thread Sebastian Benoit
Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> Hi!
> 
> On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > I guess that this would need strcasestr() instead of strcasecmp(), since you
> > are looking for the substring "Upgrade" in value. Maybe more is needed if
> > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> 
> You are correct about strcasestr. "Connection: Upgrade-maybe" would need 
> to have correct "Upgrade: websocket". Anyway, lets be strict.
> 
> Does something like this make sense?

i think the seperator list needs to include '\t' 
because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.

And i dont think you can mix "," with " \t" seperators,
because otherwise "Foo Upgrade, Bar" will match.

Something more is needed to parse elements of a header.
 
> Index: usr.sbin/relayd/relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 relay_http.c
> --- usr.sbin/relayd/relay_http.c  4 Mar 2019 21:25:03 -   1.72
> +++ usr.sbin/relayd/relay_http.c  5 Mar 2019 22:33:47 -
> @@ -166,6 +166,7 @@ relay_read_http(struct bufferevent *bev,
>   struct relay_http_priv  *priv = con->se_priv;
>   char*line = NULL, *key, *value;
>   char*urlproto, *host, *path;
> + char*valuecopy, *valuepart;
>   int  action, unique, ret;
>   const char  *errstr;
>   size_t   size, linelen;
> @@ -399,10 +400,18 @@ relay_read_http(struct bufferevent *bev,
> 
>   if (cre->line != 1) {
>   if (cre->dir == RELAY_DIR_REQUEST) {
> - if (strcasecmp("Connection", key) == 0 &&
> - strcasecmp("Upgrade", value) == 0)
> - priv->http_upgrade_req |=
> - HTTP_CONNECTION_UPGRADE;
> +
> +
> + if (strcasecmp("Connection", key) == 0) {
> + valuecopy = strdup(value);
> + while ((valuepart = strsep(, ", 
> ")) != NULL)
> + if (strcasecmp("Upgrade", valuepart) == 
> 0)
> + priv->http_upgrade_req |=
> + HTTP_CONNECTION_UPGRADE;
> + free(valuecopy);
> + }
> +
> +
>   if (strcasecmp("Upgrade", key) == 0 &&
>   strcasecmp("websocket", value) == 0)
>   priv->http_upgrade_req |=
> 
> 
> 
> begin-base64 644 websocket2.diff
> SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09
> PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog
> L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp
> b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5
> ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp
> bi9yZWxheWQvcmVsYXlfaHR0cC5jCTUgTWFyIDIwMTkgMjI6MzM6NDcgLTAwMDAKQEAgLTE2Niw2
> ICsxNjYsNyBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpiZXYsCiAJc3Ry
> dWN0IHJlbGF5X2h0dHBfcHJpdgkqcHJpdiA9IGNvbi0+c2VfcHJpdjsKIAljaGFyCQkJKmxpbmUg
> PSBOVUxMLCAqa2V5LCAqdmFsdWU7CiAJY2hhcgkJCSp1cmxwcm90bywgKmhvc3QsICpwYXRoOwor
> CWNoYXIJCQkqdmFsdWVjb3B5LCAqdmFsdWVwYXJ0OwogCWludAkJCSBhY3Rpb24sIHVuaXF1ZSwg
> cmV0OwogCWNvbnN0IGNoYXIJCSplcnJzdHI7CiAJc2l6ZV90CQkJIHNpemUsIGxpbmVsZW47CkBA
> IC0zOTksMTAgKzQwMCwxOCBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpi
> ZXYsCiAKIAkJaWYgKGNyZS0+bGluZSAhPSAxKSB7CiAJCQlpZiAoY3JlLT5kaXIgPT0gUkVMQVlf
> RElSX1JFUVVFU1QpIHsKLQkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVjdGlvbiIsIGtleSkgPT0g
> MCAmJgotCQkJCSAgICBzdHJjYXNlY21wKCJVcGdyYWRlIiwgdmFsdWUpID09IDApCi0JCQkJCXBy
> aXYtPmh0dHBfdXBncmFkZV9yZXEgfD0KLQkJCQkJICAgIEhUVFBfQ09OTkVDVElPTl9VUEdSQURF
> OworCisKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVjdGlvbiIsIGtleSkgPT0gMCkgeworCQkJ
> CSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOworCQkJCSAgICB3aGlsZSAoKHZhbHVlcGFy
> dCA9IHN0cnNlcCgmdmFsdWVjb3B5LCAiLCAiKSkgIT0gTlVMTCkKKwkJCQkgICAgCWlmIChzdHJj
> YXNlY21wKCJVcGdyYWRlIiwgdmFsdWVwYXJ0KSA9PSAwKQorCQkJCQkgICAgcHJpdi0+aHR0cF91
> cGdyYWRlX3JlcSB8PQorCQkJCQkgICAgCUhUVFBfQ09OTkVDVElPTl9VUEdSQURFOworCQkJCSAg
> ICBmcmVlKHZhbHVlY29weSk7CisJCQkJfQorCisKIAkJCQlpZiAoc3RyY2FzZWNtcCgiVXBncmFk
> ZSIsIGtleSkgPT0gMCAmJgogCQkJCSAgICBzdHJjYXNlY21wKCJ3ZWJzb2NrZXQiLCB2YWx1ZSkg
> PT0gMCkKIAkJCQkJcHJpdi0+aHR0cF91cGdyYWRlX3JlcSB8PQo=
> 
> 
> 
> 
> 
> 
> 



Re: acpithinkpad: fix brightness keys, keyboard backlight value

2019-03-06 Thread Stefan Sperling
On Tue, Mar 05, 2019 at 02:03:13PM -0600, joshua stein wrote:
> 0. "newer machines" being those with MHKV reporting version 2.  If 
> this diff breaks on older "newer machines", this metric will have to 
> be changed to something else.

Works fine here on x201 and x250.



Re: acpithinkpad: fix brightness keys, keyboard backlight value

2019-03-06 Thread Claudio Jeker
On Tue, Mar 05, 2019 at 02:03:13PM -0600, joshua stein wrote:
> Here we go again...
> 
> On at least the ThinkPad X1C6, the screen brightness keys (F5 and 
> F6) do not work and "wsconsctl keyboard.backlight" doesn't report 
> the correct value when the keyboard backlight is adjusted with 
> Fn+Space.
> 
> These are both caused by the default event mask not including these 
> events, so explicitly enable them.
> 
> But then acpithinkpad has to actually do something for the screen 
> brightness keys, but it tries the very old CMOS method which doesn't 
> work on these newer machines[0].  So make it use the ACPI method.
> 
> I renamed thinkpad_[gs]et_backlight to thinkpad_[gs]et_kbd_backlight 
> because it was confusing that they have nothing to do with screen 
> backlight.
> 
> 
> 0. "newer machines" being those with MHKV reporting version 2.  If 
> this diff breaks on older "newer machines", this metric will have to 
> be changed to something else.
> 

Tested on an x240 and x270. Both work fine (actually fixes the x270).
Diff reads ok to me. OK claudio@
 
> Index: sys/dev/acpi/acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.61
> diff -u -p -u -p -r1.61 acpithinkpad.c
> --- sys/dev/acpi/acpithinkpad.c   1 Jul 2018 19:40:49 -   1.61
> +++ sys/dev/acpi/acpithinkpad.c   5 Mar 2019 20:00:23 -
> @@ -124,6 +124,10 @@
>  #define  THINKPAD_ADAPTIVE_MODE_HOME 1
>  #define  THINKPAD_ADAPTIVE_MODE_FUNCTION 3
>  
> +#define THINKPAD_MASK_BRIGHTNESS_UP  (1 << 15)
> +#define THINKPAD_MASK_BRIGHTNESS_DOWN(1 << 16)
> +#define THINKPAD_MASK_KBD_BACKLIGHT  (1 << 17)
> +
>  struct acpithinkpad_softc {
>   struct devicesc_dev;
>  
> @@ -134,6 +138,8 @@ struct acpithinkpad_softc {
>   struct ksensor   sc_sens[THINKPAD_NSENSORS];
>   struct ksensordevsc_sensdev;
>  
> + uint64_t sc_hkey_version;
> +
>   uint64_t sc_thinklight;
>   const char  *sc_thinklight_get;
>   const char  *sc_thinklight_set;
> @@ -161,8 +167,8 @@ int   thinkpad_activate(struct device *, i
>  /* wscons hook functions */
>  void thinkpad_get_thinklight(struct acpithinkpad_softc *);
>  void thinkpad_set_thinklight(void *, int);
> -int  thinkpad_get_backlight(struct wskbd_backlight *);
> -int  thinkpad_set_backlight(struct wskbd_backlight *);
> +int  thinkpad_get_kbd_backlight(struct wskbd_backlight *);
> +int  thinkpad_set_kbd_backlight(struct wskbd_backlight *);
>  extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
>  extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
>  void thinkpad_get_brightness(struct acpithinkpad_softc *);
> @@ -284,6 +290,10 @@ thinkpad_attach(struct device *parent, s
>  
>   printf("\n");
>  
> + if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKV", 0, NULL,
> + >sc_hkey_version))
> + sc->sc_hkey_version = THINKPAD_HKEY_VERSION1;
> +
>  #if NAUDIO > 0 && NWSKBD > 0
>   /* Defer speaker mute */
>   if (thinkpad_get_volume_mute(sc) == 1)
> @@ -299,14 +309,14 @@ thinkpad_attach(struct device *parent, s
>   0, NULL, >sc_thinklight) == 0) {
>   sc->sc_thinklight_get = "KLCG";
>   sc->sc_thinklight_set = "KLCS";
> - wskbd_get_backlight = thinkpad_get_backlight;
> - wskbd_set_backlight = thinkpad_set_backlight;
> + wskbd_get_backlight = thinkpad_get_kbd_backlight;
> + wskbd_set_backlight = thinkpad_set_kbd_backlight;
>   } else if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MLCG",
>   0, NULL, >sc_thinklight) == 0) {
>   sc->sc_thinklight_get = "MLCG";
>   sc->sc_thinklight_set = "MLCS";
> - wskbd_get_backlight = thinkpad_get_backlight;
> - wskbd_set_backlight = thinkpad_set_backlight;
> + wskbd_get_backlight = thinkpad_get_kbd_backlight;
> + wskbd_set_backlight = thinkpad_set_kbd_backlight;
>   }
>  
>   if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG",
> @@ -327,13 +337,19 @@ thinkpad_enable_events(struct acpithinkp
>   int64_t mask;
>   int i;
>  
> - /* Get the supported event mask */
> + /* Get the default event mask */
>   if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKA",
>   0, NULL, )) {
>   printf("%s: no MHKA\n", DEVNAME(sc));
>   return (1);
>   }
>  
> + /* Enable events we need to know about */
> + mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
> + THINKPAD_MASK_KBD_BACKLIGHT);
> +
> + DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
> +
>   /* Update hotkey mask */
>   bzero(args, sizeof(args));
>   args[0].type = args[1].type = AML_OBJTYPE_INTEGER;
> @@ -380,6 +396,8 @@