From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/atomic: fix out of bounds read in for_each_*_in_state helpers Date: Mon, 25 May 2015 17:03:25 +0300 Message-ID: <87fv6kiwk2.fsf@intel.com> References: <1432549784-21966-1-git-send-email-a.ryabinin@samsung.com> <87oal8iyxh.fsf@intel.com> <556322E6.3020500@samsung.com> <87lhgcix4s.fsf@intel.com> <87iobgix28.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 5EE766E480 for ; Mon, 25 May 2015 07:01:17 -0700 (PDT) In-Reply-To: <87iobgix28.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Andrey Ryabinin , David Airlie Cc: Ander Conselvan de Oliveira , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gTW9uLCAyNSBNYXkgMjAxNSwgSmFuaSBOaWt1bGEgPGphbmkubmlrdWxhQGxpbnV4LmludGVs LmNvbT4gd3JvdGU6Cj4gT24gTW9uLCAyNSBNYXkgMjAxNSwgSmFuaSBOaWt1bGEgPGphbmkubmlr dWxhQGxpbnV4LmludGVsLmNvbT4gd3JvdGU6Cj4+IE9uIE1vbiwgMjUgTWF5IDIwMTUsIEFuZHJl eSBSeWFiaW5pbiA8YS5yeWFiaW5pbkBzYW1zdW5nLmNvbT4gd3JvdGU6Cj4+PiBPbiAwNS8yNS8y MDE1IDA0OjEyIFBNLCBKYW5pIE5pa3VsYSB3cm90ZToKPj4+PiBPbiBNb24sIDI1IE1heSAyMDE1 LCBBbmRyZXkgUnlhYmluaW4gPGEucnlhYmluaW5Ac2Ftc3VuZy5jb20+IHdyb3RlOgo+Pj4+PiBm b3JfZWFjaF8qX2luX3N0YXRlIHZhbGlkYXRlIGFycmF5IGluZGV4IGFmdGVyCj4+Pj4+IGFjY2Vz cyB0byBhcnJheSBlbGVtZW50cywgdGh1cyBwZXJmb3JtIG91dCBvZiBib3VuZHMgcmVhZC4KPj4+ Pj4KPj4+Pj4gRml4IHRoaXMgYnkgdmFsaWRhdGluZyBpbmRleCBpbiB0aGUgZmlyc3QgcGxhY2Ug YW5kIHJlYWQKPj4+Pj4gYXJyYXkgZWxlbWVudCBpZmYgdmFsaWRhdGlvbiB3YXMgc3VjY2Vzc2Z1 bC4KPj4+Pj4KPj4+Pj4gRml4ZXM6IGRmNjNiOTk5NGVhZiAoImRybS9hdG9taWM6IEFkZCBmb3Jf ZWFjaF97Y29ubmVjdG9yLGNydGMscGxhbmV9X2luX3N0YXRlIGhlbHBlciBtYWNyb3MiKQo+Pj4+ PiBTaWduZWQtb2ZmLWJ5OiBBbmRyZXkgUnlhYmluaW4gPGEucnlhYmluaW5Ac2Ftc3VuZy5jb20+ Cj4+Pj4+IC0tLQo+Pj4+PiAgaW5jbHVkZS9kcm0vZHJtX2F0b21pYy5oIHwgMjQgKysrKysrKysr KysrLS0tLS0tLS0tLS0tCj4+Pj4+ICAxIGZpbGUgY2hhbmdlZCwgMTIgaW5zZXJ0aW9ucygrKSwg MTIgZGVsZXRpb25zKC0pCj4+Pj4+Cj4+Pj4+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2RybS9kcm1f YXRvbWljLmggYi9pbmNsdWRlL2RybS9kcm1fYXRvbWljLmgKPj4+Pj4gaW5kZXggYzE1NzEwMzQu LjNmMTNiOTEgMTAwNjQ0Cj4+Pj4+IC0tLSBhL2luY2x1ZGUvZHJtL2RybV9hdG9taWMuaAo+Pj4+ PiArKysgYi9pbmNsdWRlL2RybS9kcm1fYXRvbWljLmgKPj4+Pj4gQEAgLTc3LDI2ICs3NywyNiBA QCBpbnQgX19tdXN0X2NoZWNrIGRybV9hdG9taWNfYXN5bmNfY29tbWl0KHN0cnVjdCBkcm1fYXRv bWljX3N0YXRlICpzdGF0ZSk7Cj4+Pj4+ICAKPj4+Pj4gICNkZWZpbmUgZm9yX2VhY2hfY29ubmVj dG9yX2luX3N0YXRlKHN0YXRlLCBjb25uZWN0b3IsIGNvbm5lY3Rvcl9zdGF0ZSwgX19pKSBcCj4+ Pj4+ICAJZm9yICgoX19pKSA9IDA7CQkJCQkJCVwKPj4+Pj4gLQkgICAgIChjb25uZWN0b3IpID0g KHN0YXRlKS0+Y29ubmVjdG9yc1tfX2ldLAkJCVwKPj4+Pj4gLQkgICAgIChjb25uZWN0b3Jfc3Rh dGUpID0gKHN0YXRlKS0+Y29ubmVjdG9yX3N0YXRlc1tfX2ldLAlcCj4+Pj4+IC0JICAgICAoX19p KSA8IChzdGF0ZSktPm51bV9jb25uZWN0b3I7CQkJCVwKPj4+Pj4gKwkgICAgIChfX2kpIDwgKHN0 YXRlKS0+bnVtX2Nvbm5lY3RvciAmJgkJCQlcCj4+Pj4+ICsJICAgICAoKGNvbm5lY3RvcikgPSAo c3RhdGUpLT5jb25uZWN0b3JzW19faV0sCQkJXAo+Pj4+PiArCSAgICAgKGNvbm5lY3Rvcl9zdGF0 ZSkgPSAoc3RhdGUpLT5jb25uZWN0b3Jfc3RhdGVzW19faV0sIDEpOyAJXAo+Pj4+IAo+Pj4+IFRo aXMgd2lsbCBzdG9wIGF0IHRoZSBmaXJzdCBOVUxMIGNvbm5lY3Rvci9jb25uZWN0b3Jfc3RhdGUu IFNpbWlsYXJseQo+Pj4+IGZvciB0aGUgbG9vcHMgYmVsb3cuCj4+Pj4gCj4+Pgo+Pj4gVGhpcyB3 aWxsIHN0b3AgaWZmIChfX2kpID49IChzdGF0ZSktPm51bV9jb25uZWN0b3IsIGJlY2F1c2UgdGhl IHJlc3VsdCBvZiBleHByZXNzaW9uOgo+Pj4gCSAoKGNvbm5lY3RvcikgPSAoc3RhdGUpLT5jb25u ZWN0b3JzW19faV0sIChjb25uZWN0b3Jfc3RhdGUpID0gKHN0YXRlKS0+Y29ubmVjdG9yX3N0YXRl c1tfX2ldLCAxKQo+Pj4gaXMgYWx3YXlzIDEuCj4+Cj4+IFdoeSBkbyB5b3UgdGhpbmsgaXQnbGwg YWx3YXlzIGJlIDE/Cj4KPiBUaGF0IG1pZ2h0IGJlIGJlY2F1c2UgdGhlcmUncyB0aGUgMSBhdCB0 aGUgZW5kLiAqYmx1c2gqLgo+Cj4gSSBkbyB3b25kZXIgaWYgdGhpcyBpcyB0b28gc3VidGxlIGlu IGdlbmVyYWwsIG9yIGlmIGl0J3MganVzdCB0b28gc3VidGxlCj4gZm9yIG1lLgoKClNvIGluIHRo ZSBtZWFuIHRpbWUsIEkgd2FzIGxvb2tpbmcgYXQgZG9pbmcgdGhlIGJlbG93LCBub3QgYmVjYXVz ZSBvZgp0aGlzIHBhdGNoIG9yIHRoZSBidWcgaXQgZml4ZXMsIGJ1dCBiZWNhdXNlIEkgdGhpbmsg dGhlIGNvbnN0cnVjdAoKCWZvciAoLi4uKSBcCiAgICAgICAgCWlmICguLi4pCgppbiBhIGZvcl9z b21ldGhpbmcoKSBzdHlsZSBtYWNybyBpcyBhIGRhbmdsaW5nIGVsc2UgZGlzYXN0ZXIgd2FpdGlu ZyB0bwpoYXBwZW4uCgpJdCdzIGEgYml0IHRlZGlvdXMsIEkgYWRtaXQsIGFuZCBhcHBhcmVudGx5 IG1ha2VzIHNvbWUgZ2NjIHZlcnNpb25zCndoaW5lIGFib3V0IHVzaW5nIHVuaW5pdGlhbGl6ZWQg dmFyaWFibGVzLCBiZWNhdXNlIHRoZXkncmUgbm90IHNtYXJ0CmVub3VnaCB0byByZWFsaXplIHRo ZSBwb2ludGVycyBhcmUgaW5pdGlhbGl6ZWQgd2hlbiB0aGUgbG9vcCBjb25kaXRpb24KaXMgbWV0 LgoKQlIsCkphbmkuCgoKZGlmZiAtLWdpdCBhL2luY2x1ZGUvZHJtL2RybV9hdG9taWMuaCBiL2lu Y2x1ZGUvZHJtL2RybV9hdG9taWMuaAppbmRleCBlODlkYjBjMzc3YmEuLmViODFmNTkzMGE4YyAx MDA2NDQKLS0tIGEvaW5jbHVkZS9kcm0vZHJtX2F0b21pYy5oCisrKyBiL2luY2x1ZGUvZHJtL2Ry bV9hdG9taWMuaApAQCAtMTM0LDI4ICsxMzQsNzAgQEAgaW50IF9fbXVzdF9jaGVjayBkcm1fYXRv bWljX2NoZWNrX29ubHkoc3RydWN0IGRybV9hdG9taWNfc3RhdGUgKnN0YXRlKTsKIGludCBfX211 c3RfY2hlY2sgZHJtX2F0b21pY19jb21taXQoc3RydWN0IGRybV9hdG9taWNfc3RhdGUgKnN0YXRl KTsKIGludCBfX211c3RfY2hlY2sgZHJtX2F0b21pY19hc3luY19jb21taXQoc3RydWN0IGRybV9h dG9taWNfc3RhdGUgKnN0YXRlKTsKIAorc3RhdGljIGlubGluZSBpbnQgbmV4dF9jb25uZWN0b3Io c3RydWN0IGRybV9jb25uZWN0b3IgKipwY29ubmVjdG9yLAorCQkJCSBzdHJ1Y3QgZHJtX2Nvbm5l Y3RvciAqKmNvbm5lY3RvcnMsCisJCQkJIHN0cnVjdCBkcm1fY29ubmVjdG9yX3N0YXRlICoqcHN0 YXRlLAorCQkJCSBzdHJ1Y3QgZHJtX2Nvbm5lY3Rvcl9zdGF0ZSAqKnN0YXRlcywKKwkJCQkgaW50 IGluZGV4LCBpbnQgbWF4KQoreworCXdoaWxlIChpbmRleCA8IG1heCAmJiAoY29ubmVjdG9yc1tp bmRleF0gPT0gTlVMTCB8fCBzdGF0ZXNbaW5kZXhdID09IE5VTEwpKQorCQlpbmRleCsrOworCisJ aWYgKGluZGV4IDwgbWF4KSB7CisJCSpwY29ubmVjdG9yID0gY29ubmVjdG9yc1tpbmRleF07CisJ CSpwc3RhdGUgPSBzdGF0ZXNbaW5kZXhdOworCX0KKworCXJldHVybiBpbmRleDsKK30KKworc3Rh dGljIGlubGluZSBpbnQgbmV4dF9jcnRjKHN0cnVjdCBkcm1fY3J0YyAqKnBjcnRjLAorCQkJICAg IHN0cnVjdCBkcm1fY3J0YyAqKmNydGNzLAorCQkJICAgIHN0cnVjdCBkcm1fY3J0Y19zdGF0ZSAq KnBzdGF0ZSwKKwkJCSAgICBzdHJ1Y3QgZHJtX2NydGNfc3RhdGUgKipzdGF0ZXMsCisJCQkgICAg aW50IGluZGV4LCBpbnQgbWF4KQoreworCXdoaWxlIChpbmRleCA8IG1heCAmJiAoY3J0Y3NbaW5k ZXhdID09IE5VTEwgfHwgc3RhdGVzW2luZGV4XSA9PSBOVUxMKSkKKwkJaW5kZXgrKzsKKworCWlm IChpbmRleCA8IG1heCkgeworCQkqcGNydGMgPSBjcnRjc1tpbmRleF07CisJCSpwc3RhdGUgPSBz dGF0ZXNbaW5kZXhdOworCX0KKworCXJldHVybiBpbmRleDsKK30KKworc3RhdGljIGlubGluZSBp bnQgbmV4dF9wbGFuZShzdHJ1Y3QgZHJtX3BsYW5lICoqcHBsYW5lLAorCQkJICAgIHN0cnVjdCBk cm1fcGxhbmUgKipwbGFuZXMsCisJCQkgICAgc3RydWN0IGRybV9wbGFuZV9zdGF0ZSAqKnBzdGF0 ZSwKKwkJCSAgICBzdHJ1Y3QgZHJtX3BsYW5lX3N0YXRlICoqc3RhdGVzLAorCQkJICAgIGludCBp bmRleCwgaW50IG1heCkKK3sKKwl3aGlsZSAoaW5kZXggPCBtYXggJiYgKHBsYW5lc1tpbmRleF0g PT0gTlVMTCB8fCBzdGF0ZXNbaW5kZXhdID09IE5VTEwpKQorCQlpbmRleCsrOworCisJaWYgKGlu ZGV4IDwgbWF4KSB7CisJCSpwcGxhbmUgPSBwbGFuZXNbaW5kZXhdOworCQkqcHN0YXRlID0gc3Rh dGVzW2luZGV4XTsKKwl9CisKKwlyZXR1cm4gaW5kZXg7Cit9CisKICNkZWZpbmUgZm9yX2VhY2hf Y29ubmVjdG9yX2luX3N0YXRlKHN0YXRlLCBjb25uZWN0b3IsIGNvbm5lY3Rvcl9zdGF0ZSwgX19p KSBcCi0JZm9yICgoX19pKSA9IDA7CQkJCQkJCVwKLQkgICAgIChjb25uZWN0b3IpID0gKHN0YXRl KS0+Y29ubmVjdG9yc1tfX2ldLAkJCVwKLQkgICAgIChjb25uZWN0b3Jfc3RhdGUpID0gKHN0YXRl KS0+Y29ubmVjdG9yX3N0YXRlc1tfX2ldLAlcCisJZm9yICgoX19pKSA9IG5leHRfY29ubmVjdG9y KCYoY29ubmVjdG9yKSwgKHN0YXRlKS0+Y29ubmVjdG9ycywgJihjb25uZWN0b3Jfc3RhdGUpLCAo c3RhdGUpLT5jb25uZWN0b3Jfc3RhdGVzLCAwLCAoc3RhdGUpLT5udW1fY29ubmVjdG9yKTsgXAog CSAgICAgKF9faSkgPCAoc3RhdGUpLT5udW1fY29ubmVjdG9yOwkJCQlcCi0JICAgICAoX19pKSsr KQkJCQkJCQlcCi0JCWlmIChjb25uZWN0b3IpCi0KLSNkZWZpbmUgZm9yX2VhY2hfY3J0Y19pbl9z dGF0ZShzdGF0ZSwgY3J0YywgY3J0Y19zdGF0ZSwgX19pKQlcCi0JZm9yICgoX19pKSA9IDA7CQkJ CQkJXAotCSAgICAgKGNydGMpID0gKHN0YXRlKS0+Y3J0Y3NbX19pXSwJCQlcCi0JICAgICAoY3J0 Y19zdGF0ZSkgPSAoc3RhdGUpLT5jcnRjX3N0YXRlc1tfX2ldLAkJXAotCSAgICAgKF9faSkgPCAo c3RhdGUpLT5kZXYtPm1vZGVfY29uZmlnLm51bV9jcnRjOwlcCi0JICAgICAoX19pKSsrKQkJCQkJ CVwKLQkJaWYgKGNydGNfc3RhdGUpCi0KLSNkZWZpbmUgZm9yX2VhY2hfcGxhbmVfaW5fc3RhdGUo c3RhdGUsIHBsYW5lLCBwbGFuZV9zdGF0ZSwgX19pKQlcCi0JZm9yICgoX19pKSA9IDA7CQkJCQkJ XAotCSAgICAgKHBsYW5lKSA9IChzdGF0ZSktPnBsYW5lc1tfX2ldLAkJCVwKLQkgICAgIChwbGFu ZV9zdGF0ZSkgPSAoc3RhdGUpLT5wbGFuZV9zdGF0ZXNbX19pXSwJXAotCSAgICAgKF9faSkgPCAo c3RhdGUpLT5kZXYtPm1vZGVfY29uZmlnLm51bV90b3RhbF9wbGFuZTsJXAotCSAgICAgKF9faSkr KykJCQkJCQlcCi0JCWlmIChwbGFuZV9zdGF0ZSkKKwkgICAgIChfX2kpID0gbmV4dF9jb25uZWN0 b3IoJihjb25uZWN0b3IpLCAoc3RhdGUpLT5jb25uZWN0b3JzLCAmKGNvbm5lY3Rvcl9zdGF0ZSks IChzdGF0ZSktPmNvbm5lY3Rvcl9zdGF0ZXMsIChfX2kpLCAoc3RhdGUpLT5udW1fY29ubmVjdG9y KSkKKworI2RlZmluZSBmb3JfZWFjaF9jcnRjX2luX3N0YXRlKHN0YXRlLCBjcnRjLCBjcnRjX3N0 YXRlLCBfX2kpCQlcCisJZm9yICgoX19pKSA9IG5leHRfY3J0YygmKGNydGMpLCAoc3RhdGUpLT5j cnRjcywgJihjcnRjX3N0YXRlKSwgKHN0YXRlKS0+Y3J0Y19zdGF0ZXMsIDAsIChzdGF0ZSktPmRl di0+bW9kZV9jb25maWcubnVtX2NydGMpOyBcCisJICAgICAoX19pKSA8IChzdGF0ZSktPmRldi0+ bW9kZV9jb25maWcubnVtX2NydGM7CQlcCisJICAgICAoX19pKSA9IG5leHRfY3J0YygmY3J0Yywg KHN0YXRlKS0+Y3J0Y3MsICYoY3J0Y19zdGF0ZSksIChzdGF0ZSktPmNydGNfc3RhdGVzLCAoX19p KSwgKHN0YXRlKS0+ZGV2LT5tb2RlX2NvbmZpZy5udW1fY3J0YykpCisKKyNkZWZpbmUgZm9yX2Vh Y2hfcGxhbmVfaW5fc3RhdGUoc3RhdGUsIHBsYW5lLCBwbGFuZV9zdGF0ZSwgX19pKQkJXAorCWZv ciAoKF9faSkgPSBuZXh0X3BsYW5lKCYocGxhbmUpLCAoc3RhdGUpLT5wbGFuZXMsICYocGxhbmVf c3RhdGUpLCAoc3RhdGUpLT5wbGFuZV9zdGF0ZXMsIDAsIChzdGF0ZSktPmRldi0+bW9kZV9jb25m aWcubnVtX3RvdGFsX3BsYW5lKTsgXAorCSAgICAgKF9faSkgPCAoc3RhdGUpLT5kZXYtPm1vZGVf Y29uZmlnLm51bV90b3RhbF9wbGFuZTsJCVwKKwkgICAgIChfX2kpID0gbmV4dF9wbGFuZSgmKHBs YW5lKSwgKHN0YXRlKS0+cGxhbmVzLCAmKHBsYW5lX3N0YXRlKSwgKHN0YXRlKS0+cGxhbmVfc3Rh dGVzLCAoX19pKSwgKHN0YXRlKS0+ZGV2LT5tb2RlX2NvbmZpZy5udW1fdG90YWxfcGxhbmUpKQog CiAjZW5kaWYgLyogRFJNX0FUT01JQ19IXyAqLwoKCgoKLS0gCkphbmkgTmlrdWxhLCBJbnRlbCBP cGVuIFNvdXJjZSBUZWNobm9sb2d5IENlbnRlcgpfX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0 cy5mcmVlZGVza3RvcC5vcmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xp c3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751471AbbEYOBU (ORCPT ); Mon, 25 May 2015 10:01:20 -0400 Received: from mga09.intel.com ([134.134.136.24]:28166 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751001AbbEYOBR (ORCPT ); Mon, 25 May 2015 10:01:17 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,491,1427785200"; d="scan'208";a="731355557" From: Jani Nikula To: Andrey Ryabinin , David Airlie Cc: Ander Conselvan de Oliveira , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/atomic: fix out of bounds read in for_each_*_in_state helpers In-Reply-To: <87iobgix28.fsf@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <1432549784-21966-1-git-send-email-a.ryabinin@samsung.com> <87oal8iyxh.fsf@intel.com> <556322E6.3020500@samsung.com> <87lhgcix4s.fsf@intel.com> <87iobgix28.fsf@intel.com> User-Agent: Notmuch/0.19+112~g77230b0 (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Mon, 25 May 2015 17:03:25 +0300 Message-ID: <87fv6kiwk2.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 25 May 2015, Jani Nikula wrote: > On Mon, 25 May 2015, Jani Nikula wrote: >> On Mon, 25 May 2015, Andrey Ryabinin wrote: >>> On 05/25/2015 04:12 PM, Jani Nikula wrote: >>>> On Mon, 25 May 2015, Andrey Ryabinin wrote: >>>>> for_each_*_in_state validate array index after >>>>> access to array elements, thus perform out of bounds read. >>>>> >>>>> Fix this by validating index in the first place and read >>>>> array element iff validation was successful. >>>>> >>>>> Fixes: df63b9994eaf ("drm/atomic: Add for_each_{connector,crtc,plane}_in_state helper macros") >>>>> Signed-off-by: Andrey Ryabinin >>>>> --- >>>>> include/drm/drm_atomic.h | 24 ++++++++++++------------ >>>>> 1 file changed, 12 insertions(+), 12 deletions(-) >>>>> >>>>> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h >>>>> index c1571034..3f13b91 100644 >>>>> --- a/include/drm/drm_atomic.h >>>>> +++ b/include/drm/drm_atomic.h >>>>> @@ -77,26 +77,26 @@ int __must_check drm_atomic_async_commit(struct drm_atomic_state *state); >>>>> >>>>> #define for_each_connector_in_state(state, connector, connector_state, __i) \ >>>>> for ((__i) = 0; \ >>>>> - (connector) = (state)->connectors[__i], \ >>>>> - (connector_state) = (state)->connector_states[__i], \ >>>>> - (__i) < (state)->num_connector; \ >>>>> + (__i) < (state)->num_connector && \ >>>>> + ((connector) = (state)->connectors[__i], \ >>>>> + (connector_state) = (state)->connector_states[__i], 1); \ >>>> >>>> This will stop at the first NULL connector/connector_state. Similarly >>>> for the loops below. >>>> >>> >>> This will stop iff (__i) >= (state)->num_connector, because the result of expression: >>> ((connector) = (state)->connectors[__i], (connector_state) = (state)->connector_states[__i], 1) >>> is always 1. >> >> Why do you think it'll always be 1? > > That might be because there's the 1 at the end. *blush*. > > I do wonder if this is too subtle in general, or if it's just too subtle > for me. So in the mean time, I was looking at doing the below, not because of this patch or the bug it fixes, but because I think the construct for (...) \ if (...) in a for_something() style macro is a dangling else disaster waiting to happen. It's a bit tedious, I admit, and apparently makes some gcc versions whine about using uninitialized variables, because they're not smart enough to realize the pointers are initialized when the loop condition is met. BR, Jani. diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index e89db0c377ba..eb81f5930a8c 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -134,28 +134,70 @@ int __must_check drm_atomic_check_only(struct drm_atomic_state *state); int __must_check drm_atomic_commit(struct drm_atomic_state *state); int __must_check drm_atomic_async_commit(struct drm_atomic_state *state); +static inline int next_connector(struct drm_connector **pconnector, + struct drm_connector **connectors, + struct drm_connector_state **pstate, + struct drm_connector_state **states, + int index, int max) +{ + while (index < max && (connectors[index] == NULL || states[index] == NULL)) + index++; + + if (index < max) { + *pconnector = connectors[index]; + *pstate = states[index]; + } + + return index; +} + +static inline int next_crtc(struct drm_crtc **pcrtc, + struct drm_crtc **crtcs, + struct drm_crtc_state **pstate, + struct drm_crtc_state **states, + int index, int max) +{ + while (index < max && (crtcs[index] == NULL || states[index] == NULL)) + index++; + + if (index < max) { + *pcrtc = crtcs[index]; + *pstate = states[index]; + } + + return index; +} + +static inline int next_plane(struct drm_plane **pplane, + struct drm_plane **planes, + struct drm_plane_state **pstate, + struct drm_plane_state **states, + int index, int max) +{ + while (index < max && (planes[index] == NULL || states[index] == NULL)) + index++; + + if (index < max) { + *pplane = planes[index]; + *pstate = states[index]; + } + + return index; +} + #define for_each_connector_in_state(state, connector, connector_state, __i) \ - for ((__i) = 0; \ - (connector) = (state)->connectors[__i], \ - (connector_state) = (state)->connector_states[__i], \ + for ((__i) = next_connector(&(connector), (state)->connectors, &(connector_state), (state)->connector_states, 0, (state)->num_connector); \ (__i) < (state)->num_connector; \ - (__i)++) \ - if (connector) - -#define for_each_crtc_in_state(state, crtc, crtc_state, __i) \ - for ((__i) = 0; \ - (crtc) = (state)->crtcs[__i], \ - (crtc_state) = (state)->crtc_states[__i], \ - (__i) < (state)->dev->mode_config.num_crtc; \ - (__i)++) \ - if (crtc_state) - -#define for_each_plane_in_state(state, plane, plane_state, __i) \ - for ((__i) = 0; \ - (plane) = (state)->planes[__i], \ - (plane_state) = (state)->plane_states[__i], \ - (__i) < (state)->dev->mode_config.num_total_plane; \ - (__i)++) \ - if (plane_state) + (__i) = next_connector(&(connector), (state)->connectors, &(connector_state), (state)->connector_states, (__i), (state)->num_connector)) + +#define for_each_crtc_in_state(state, crtc, crtc_state, __i) \ + for ((__i) = next_crtc(&(crtc), (state)->crtcs, &(crtc_state), (state)->crtc_states, 0, (state)->dev->mode_config.num_crtc); \ + (__i) < (state)->dev->mode_config.num_crtc; \ + (__i) = next_crtc(&crtc, (state)->crtcs, &(crtc_state), (state)->crtc_states, (__i), (state)->dev->mode_config.num_crtc)) + +#define for_each_plane_in_state(state, plane, plane_state, __i) \ + for ((__i) = next_plane(&(plane), (state)->planes, &(plane_state), (state)->plane_states, 0, (state)->dev->mode_config.num_total_plane); \ + (__i) < (state)->dev->mode_config.num_total_plane; \ + (__i) = next_plane(&(plane), (state)->planes, &(plane_state), (state)->plane_states, (__i), (state)->dev->mode_config.num_total_plane)) #endif /* DRM_ATOMIC_H_ */ -- Jani Nikula, Intel Open Source Technology Center