linux-kernel - Re: [PATCH 1/1] Blackfin OTP Char Driver: add writing support of OTP

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:	Mon, 13 Oct 2008 11:59:21 +0200
From:	Jiri Slaby <jirislaby@...il.com>
To:	Mike Frysinger <vapier.adi@...il.com>
CC:	Bryan Wu <cooloney@...nel.org>, sam@...nborg.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] Blackfin OTP Char Driver: add writing support of
 OTP

On 10/13/2008 11:43 AM, Mike Frysinger wrote:
> On Mon, Oct 13, 2008 at 05:37, Jiri Slaby wrote:
>> On 10/13/2008 11:13 AM, Bryan Wu wrote:
>>> @@ -123,18 +132,95 @@ static ssize_t bfin_otp_write(struct file *filp, const char __user *buff, size_t
>>>       if (mutex_lock_interruptible(&bfin_otp_lock))
>>>               return -ERESTARTSYS;
>>>
>>> -     /* need otp_init() documentation before this can be implemented */
>>> +     stampit();
>>> +
>>> +     timing = bfin_otp_init_timing();
>>> +     if (timing == 0) {
>>> +             mutex_unlock(&bfin_otp_lock);
>>> +             return -EIO;
>>> +     }
>>> +
>>> +     base_flags = OTP_CHECK_FOR_PREV_WRITE;
>>> +
>>> +     bytes_done = 0;
>>> +     page = *pos / (sizeof(u64) * 2);
>>> +     while (bytes_done < count) {
>>> +             flags = base_flags | (*pos % (sizeof(u64) * 2) ? OTP_UPPER_HALF : OTP_LOWER_HALF);
>>> +             stamp("processing page %i (0x%x:%s) from %p", page, flags,
>>> +                     (flags & OTP_UPPER_HALF ? "upper" : "lower"), buff + bytes_done);
>>> +             if (copy_from_user(&content, buff + bytes_done, sizeof(content))) {
>>> +                     bytes_done = -EFAULT;
>>> +                     break;
>>> +             }
>>> +             ret = bfrom_OtpWrite(page, flags, &content);
>>> +             if (ret & OTP_MASTER_ERROR) {
>>> +                     stamp("error from otp: 0x%x", ret);
>>> +                     bytes_done = -EIO;
>>> +                     break;
>>> +             }
>>> +             if (flags & OTP_UPPER_HALF)
>>> +                     ++page;
>>> +             bytes_done += sizeof(content);
>>> +             *pos += sizeof(content);
>> What happens to pos if it fails later?
> 
> there is no state maintained in the hardware.  the pos gets updated
> only when a half-page actually gets processed.  so there is no
> "later".

Sure there is. Next iteration of the loop. I.e. what happens if bfrom_OtpWrite
fails for the second time?

>> You should change (and check) allow_writes under the mutex anyway.
> 
> not really.  the mutex is to restrict access to the OTP hardware, not
> driver state.  because there is none.  access to allow_writes is
> atomic in the hardware anyways.

Yeah, the assignment/check is.

But is this OK to you:
PROCESS 1			PROCESS 2
lock
  set allow_writes
write
   check allow_writes
   be interrupted
				whatever
				unlock
				    unset allow_writes
				sleep
   mutex lock
   the processing...
--
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/