linux-kernel - Re: [PATCH 2/3] Add rfkill support to compal-laptop

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]

Message-Id: <1250672475.25419.7.camel@johannes.local>
Date:	Wed, 19 Aug 2009 11:01:15 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Mario Limonciello <mario_limonciello@...l.com>
Cc:	Alan Jenkins <sourcejedi.lkml@...glemail.com>,
	Marcel Holtmann <marcel@...tmann.org>,
	cezary.jackiewicz@...il.com, linux-acpi@...r.kernel.org,
	linux-kernel <linux-kernel@...r.kernel.org>,
	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>
Subject: Re: [PATCH 2/3] Add rfkill support to compal-laptop

Ah, heh, thanks Alan for pointing out there was a patch here :)

> +static void compal_rfkill_poll(struct rfkill *rfkill, void *data)
> +{
> +       unsigned long radio = (unsigned long) data;
> +       u8 result;
> +       bool hw_blocked;
> +       bool sw_blocked;
> +
> +       ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
> +
> +       hw_blocked = !(result & (KILLSWITCH_MASK | radio));

I don't quite understand the "| radio" bit since that seems to be the
soft kill bit according to rfkill_set()?

> +       sw_blocked = rfkill_set_hw_state(rfkill, hw_blocked);
> +
> +       rfkill_set_sw_state(rfkill, sw_blocked);

This is wrong. You can remove the entire part about sw_blocked, almost.

> +static int compal_rfkill_set(void *data, bool blocked)
> +{
> +       unsigned long radio = (unsigned long) data;
> +       u8 result, value;
> +
> +       ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
> +
> +       if ((result & KILLSWITCH_MASK) == 0)
> +               return -EINVAL;

Anyhow, here you reject the request to set the soft bit. I suspect you
could let it go through but it would only change the soft bit in the
BIOS, nothing else really.

Two options:
1) You can let it go though, in that case do that, and remove the sw
   block stuff from poll() completely.

2) You can't let it go through. In this case, you need to leave set as
   it is, but implement poll like this:

	sw_block = rfkill_set_hw_state(rfkill, hw_blocked);
	compal_rfkill_set(data, sw_block);

so that when the user soft-blocks the device while hard-blocked, the
soft block is still honoured after pushing the button on the laptop.

Also, I'm not entirely clear about the semantics -- you've called the
bit KILLSWITCH_MASK, but does it really control all technologies as a
hard block, i.e. it toggles both the bluetooth and wireless hard block?

johannes

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)