linux-kernel - RE: [PATCH v5 2/2] mmc: core: Support packed command for eMMC4.5 device

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:	Wed, 2 May 2012 11:35:28 -0700 (PDT)
From:	merez@...eaurora.org
To:	"Seungwon Jeon" <tgih.jun@...sung.com>
Cc:	merez@...eaurora.org, linux-mmc@...r.kernel.org,
	"'Chris Ball'" <cjb@...top.org>, linux-kernel@...r.kernel.org
Subject: RE: [PATCH v5 2/2] mmc: core: Support packed command for eMMC4.5 
     device

>> > @@ -1291,10 +1657,42 @@ static int mmc_blk_issue_rw_rq(struct
>> mmc_queue
>> *mq, struct request *rqc)
>> >  			 * A block was successfully transferred.
>> >  			 */
>> >  			mmc_blk_reset_success(md, type);
>> > -			spin_lock_irq(&md->lock);
>> > -			ret = __blk_end_request(req, 0,
>> > +
>> > +			if (mq_rq->packed_cmd != MMC_PACKED_NONE) {
>> > +				int idx = mq_rq->packed_fail_idx, i = 0;
>> > +				ret = 0;
>> > +				while (!list_empty(&mq_rq->packed_list)) {
>> > +					prq = list_entry_rq(
>> > +						mq_rq->packed_list.next);
>> > +					if (idx == i) {
>> > +						/* retry from error index */
>> > +						mq_rq->packed_num -= idx;
>> > +						mq_rq->req = prq;
>> > +						ret = 1;
>> > +						break;
>> > +					}
>> > +					list_del_init(&prq->queuelist);
>> > +					spin_lock_irq(&md->lock);
>> > +					__blk_end_request(prq, 0,
>> > +							blk_rq_bytes(prq));
>> > +					spin_unlock_irq(&md->lock);
>> > +					i++;
>> > +				}
>> > +				if (mq_rq->packed_num == MMC_PACKED_N_SINGLE) {
>> > +					prq = list_entry_rq(
>> > +						mq_rq->packed_list.next);
>> You already get the prq inside the while. There is no need to do it
>> again.
> Right, but if while loop isn't taken, then prq can be used uninitialized.
> Though that case wouldn't happen actually, we don't want to see the
> compiling error.

The loop must be taken since we are inside the case of packed commands so
the list can't be empty.

If the compiler complained, you can set prq to be the first request before
entering the loop instead of setting it again in the if that follows the
loop. It will probably be more understood.
If you decide to leave it as is, I would also add the following to the if:
+                                                mq_rq->req = prq;
+                                                ret = 1;
Otherwise it seems like there could be a bug in cases where the loop is
not taken (since prq is the only one that is set) and the code is less
understood.

Thanks,
Maya Erez
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum


--
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/