linux-kernel - Re: [linux-nfc] [PATCH 8/8] NFC: Add Intel FieldsPeak NFC solution driver

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: <CAFPB+Yf1jWX=tv+feZRonZEprjXwHdD8mMXAupdm5XrDaYvWMA@mail.gmail.com>
Date:	Wed, 1 Apr 2015 18:35:31 +0300
From:	Robert Dolca <robert.dolca@...il.com>
To:	Samuel Ortiz <sameo@...ux.intel.com>
Cc:	Robert Dolca <robert.dolca@...el.com>, linux-nfc@...ts.01.org,
	netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [linux-nfc] [PATCH 8/8] NFC: Add Intel FieldsPeak NFC solution driver

On Thu, Mar 26, 2015 at 2:30 AM, Samuel Ortiz <sameo@...ux.intel.com> wrote:
>> +     /* If a patch was applied the new version is checked */
>> +     if (patched) {
>> +             r = nci_init(ndev);
>> +             if (r)
>> +                     goto error;
>> +
>> +             r = fdp_nci_get_versions(ndev);
>> +             if (r)
>> +                     goto error;
>> +
>> +             if (info->otp_version != info->otp_patch_version ||
>> +                 info->ram_version != info->ram_patch_version) {
>> +                     pr_err("FRP firmware update failed");
>> +                     r = -EINVAL;
>> +             }
>> +     }
>> +
>> +     /* Check if the device has VSC */
>> +     if (fdp_post_fw_vsc_cfg[0]) {
>> +             /* Set the vendor specific configuration */
>> +             r = fdp_nci_set_production_data(ndev, fdp_post_fw_vsc_cfg[3],
>> +                                             &fdp_post_fw_vsc_cfg[4]);
>> +             if (r)
>> +                     goto error;
>> +     }
>> +
>> +     /* Set clock type and frequency */
>> +     r = fdp_nci_set_clock(ndev, 0, 26000);
>> +     if (r)
>> +             goto error;
> The version checking, production data setting and clock setting should
> be part of a post setup notification call. Please add an nci_dev
> notify() ops that could get called on certain events, for example when
> NCI is up. Bluetooth's HCI does something along those lines already.
> From this notification hook you could implement this post setup stage.
>
> The idea is for your setup routine to only do firmware update and
> nothing else. It will make it shorter, and thus easier to read as well.

Hi Samuel,

If the RAM patch wasn't applied successfully the device can't be used
so the setup function should fail.
If the production data (specifically the clock frequency) is not set
the device can not be used. If the user space tries to start polling
before the notification is sent the polling will fail. Having it
called later would mean introducing a race condition.

Cheers,
Robert
--
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/