From patchwork Fri Dec 15 12:39:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 849144 X-Patchwork-Delegate: boris.brezillon@free-electrons.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.infradead.org (client-ip=65.50.211.133; helo=bombadil.infradead.org; envelope-from=linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ZFQE12Cc"; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3yyqmw5j7Lz9sR8 for ; Fri, 15 Dec 2017 23:41:36 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:References: In-Reply-To:Message-Id:Date:Subject:To:From:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=nCGpN4MuICjx4DUzivJQP2T3WOK0uTmUaxh43Cr2qp8=; b=ZFQE12CcdMXOIXeFAz8IKhS/0S +ONMqLn5g1Xcr+HfebPoO7FW6E0445qgjGpRAERyyEit1Xxnm5vRzTGFEai1rW0S8b+DZaWU+gYTx bhWhywLzkh0FeR7Mg+5dbHDwXNCMZRd5HtTMYYz8DNtO90efUsOyaZu6OH5UjWgBA2A07zK4qcxxI TPDgLfTKeGTXjdqifBwDZQ0TkR7AibwCn3czeEb6eNgN4ADUl+l8njib3cZs7VYGNlHvJ5rk+30cY 8tooHCfJd3CAVeGCQ2sgsfw2/3SG8oFCrZ1daYRTFpXCOXZrJG7gCAfXgjlMEi7PSp3VEfrt5fv+J 8ScSnEow==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1ePpIw-0005Ab-LX; Fri, 15 Dec 2017 12:41:30 +0000 Received: from mail.free-electrons.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1ePpHw-00045R-H2 for linux-mtd@lists.infradead.org; Fri, 15 Dec 2017 12:40:37 +0000 Received: by mail.free-electrons.com (Postfix, from userid 110) id EFA43209CA; Fri, 15 Dec 2017 13:40:07 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on mail.free-electrons.com X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT, URIBL_BLOCKED shortcircuit=ham autolearn=disabled version=3.4.0 Received: from localhost.localdomain (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr [90.63.216.87]) by mail.free-electrons.com (Postfix) with ESMTPSA id 8CEF020964; Fri, 15 Dec 2017 13:40:07 +0100 (CET) From: Boris Brezillon To: David Woodhouse , Brian Norris , Boris Brezillon , Marek Vasut , Richard Weinberger , Cyrille Pitchen , linux-mtd@lists.infradead.org Subject: [PATCH v3 3/4] mtd: Stop directly calling master ->_xxx() hooks from mtdpart code Date: Fri, 15 Dec 2017 13:39:53 +0100 Message-Id: <20171215123954.30017-4-boris.brezillon@free-electrons.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20171215123954.30017-1-boris.brezillon@free-electrons.com> References: <20171215123954.30017-1-boris.brezillon@free-electrons.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171215_044029_158311_2A8F2C48 X-CRM114-Status: GOOD ( 16.93 ) X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.4.1 on bombadil.infradead.org summary: Content analysis details: (-1.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Pan , Kyungmin Park , Robert Jarzmik , Frieder Schrempf MIME-Version: 1.0 Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org The MTD layer provides several wrappers around mtd->_xxx() hooks. Call these wrappers instead of directly dereferencing the associated ->_xxx() pointer. This change has been motivated by another rework letting the core handle the case where ->_read/write_oob() are implemented but not ->_read/write(). In this case, we want mtd_read/write() to fall back to ->_read/write_oob() when ->_read/write() are NULL. The problem is, mtdpart is directly calling the ->_xxx() instead of using the wrappers, thus leading to a NULL pointer exception. Even though we only need to do the change for part_read/write(), going through those wrappers for all kind of part -> master operation propagation is a good thing, because other wrappers might become smarter over time, and the duplicated check overhead (parameters will be checked at the partition and master level instead of only at the partition level) should be negligible. Signed-off-by: Boris Brezillon --- Changes in v3: - unconditionally assign part wrappers as suggested by Brian Changes in v2: - new patch needed to fix a NULL pointer dereference BUG --- drivers/mtd/mtdpart.c | 141 +++++++++++++++++++------------------------------- 1 file changed, 53 insertions(+), 88 deletions(-) diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index be088bccd593..e83c9d870b11 100644 --- a/drivers/mtd/mtdpart.c +++ b/drivers/mtd/mtdpart.c @@ -74,8 +74,7 @@ static int part_read(struct mtd_info *mtd, loff_t from, size_t len, int res; stats = part->parent->ecc_stats; - res = part->parent->_read(part->parent, from + part->offset, len, - retlen, buf); + res = mtd_read(part->parent, from + part->offset, len, retlen, buf); if (unlikely(mtd_is_eccerr(res))) mtd->ecc_stats.failed += part->parent->ecc_stats.failed - stats.failed; @@ -90,15 +89,15 @@ static int part_point(struct mtd_info *mtd, loff_t from, size_t len, { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_point(part->parent, from + part->offset, len, - retlen, virt, phys); + return mtd_point(part->parent, from + part->offset, len, retlen, virt, + phys); } static int part_unpoint(struct mtd_info *mtd, loff_t from, size_t len) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_unpoint(part->parent, from + part->offset, len); + return mtd_unpoint(part->parent, from + part->offset, len); } static int part_read_oob(struct mtd_info *mtd, loff_t from, @@ -126,7 +125,7 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from, return -EINVAL; } - res = part->parent->_read_oob(part->parent, from + part->offset, ops); + res = mtd_read_oob(part->parent, from + part->offset, ops); if (unlikely(res)) { if (mtd_is_bitflip(res)) mtd->ecc_stats.corrected++; @@ -140,48 +139,43 @@ static int part_read_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, u_char *buf) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_read_user_prot_reg(part->parent, from, len, - retlen, buf); + return mtd_read_user_prot_reg(part->parent, from, len, retlen, buf); } static int part_get_user_prot_info(struct mtd_info *mtd, size_t len, size_t *retlen, struct otp_info *buf) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_get_user_prot_info(part->parent, len, retlen, - buf); + return mtd_get_user_prot_info(part->parent, len, retlen, buf); } static int part_read_fact_prot_reg(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, u_char *buf) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_read_fact_prot_reg(part->parent, from, len, - retlen, buf); + return mtd_read_fact_prot_reg(part->parent, from, len, retlen, buf); } static int part_get_fact_prot_info(struct mtd_info *mtd, size_t len, size_t *retlen, struct otp_info *buf) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_get_fact_prot_info(part->parent, len, retlen, - buf); + return mtd_get_fact_prot_info(part->parent, len, retlen, buf); } static int part_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, const u_char *buf) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_write(part->parent, to + part->offset, len, - retlen, buf); + return mtd_write(part->parent, to + part->offset, len, retlen, buf); } static int part_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, const u_char *buf) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_panic_write(part->parent, to + part->offset, len, - retlen, buf); + return mtd_panic_write(part->parent, to + part->offset, len, retlen, + buf); } static int part_write_oob(struct mtd_info *mtd, loff_t to, @@ -193,30 +187,29 @@ static int part_write_oob(struct mtd_info *mtd, loff_t to, return -EINVAL; if (ops->datbuf && to + ops->len > mtd->size) return -EINVAL; - return part->parent->_write_oob(part->parent, to + part->offset, ops); + return mtd_write_oob(part->parent, to + part->offset, ops); } static int part_write_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, u_char *buf) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_write_user_prot_reg(part->parent, from, len, - retlen, buf); + return mtd_write_user_prot_reg(part->parent, from, len, retlen, buf); } static int part_lock_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_lock_user_prot_reg(part->parent, from, len); + return mtd_lock_user_prot_reg(part->parent, from, len); } static int part_writev(struct mtd_info *mtd, const struct kvec *vecs, unsigned long count, loff_t to, size_t *retlen) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_writev(part->parent, vecs, count, - to + part->offset, retlen); + return mtd_writev(part->parent, vecs, count, to + part->offset, + retlen); } static int part_erase(struct mtd_info *mtd, struct erase_info *instr) @@ -225,7 +218,7 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr) int ret; instr->addr += part->offset; - ret = part->parent->_erase(part->parent, instr); + ret = mtd_erase(part->parent, instr); if (ret) { if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN) instr->fail_addr -= part->offset; @@ -251,51 +244,51 @@ EXPORT_SYMBOL_GPL(mtd_erase_callback); static int part_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_lock(part->parent, ofs + part->offset, len); + return mtd_lock(part->parent, ofs + part->offset, len); } static int part_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_unlock(part->parent, ofs + part->offset, len); + return mtd_unlock(part->parent, ofs + part->offset, len); } static int part_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_is_locked(part->parent, ofs + part->offset, len); + return mtd_is_locked(part->parent, ofs + part->offset, len); } static void part_sync(struct mtd_info *mtd) { struct mtd_part *part = mtd_to_part(mtd); - part->parent->_sync(part->parent); + mtd_sync(part->parent); } static int part_suspend(struct mtd_info *mtd) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_suspend(part->parent); + return mtd_suspend(part->parent); } static void part_resume(struct mtd_info *mtd) { struct mtd_part *part = mtd_to_part(mtd); - part->parent->_resume(part->parent); + mtd_resume(part->parent); } static int part_block_isreserved(struct mtd_info *mtd, loff_t ofs) { struct mtd_part *part = mtd_to_part(mtd); ofs += part->offset; - return part->parent->_block_isreserved(part->parent, ofs); + return mtd_block_isreserved(part->parent, ofs); } static int part_block_isbad(struct mtd_info *mtd, loff_t ofs) { struct mtd_part *part = mtd_to_part(mtd); ofs += part->offset; - return part->parent->_block_isbad(part->parent, ofs); + return mtd_block_isbad(part->parent, ofs); } static int part_block_markbad(struct mtd_info *mtd, loff_t ofs) @@ -304,7 +297,7 @@ static int part_block_markbad(struct mtd_info *mtd, loff_t ofs) int res; ofs += part->offset; - res = part->parent->_block_markbad(part->parent, ofs); + res = mtd_block_markbad(part->parent, ofs); if (!res) mtd->ecc_stats.badblocks++; return res; @@ -313,13 +306,13 @@ static int part_block_markbad(struct mtd_info *mtd, loff_t ofs) static int part_get_device(struct mtd_info *mtd) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_get_device(part->parent); + return __get_mtd_device(part->parent); } static void part_put_device(struct mtd_info *mtd) { struct mtd_part *part = mtd_to_part(mtd); - part->parent->_put_device(part->parent); + __put_mtd_device(part->parent); } static int part_ooblayout_ecc(struct mtd_info *mtd, int section, @@ -347,8 +340,7 @@ static int part_max_bad_blocks(struct mtd_info *mtd, loff_t ofs, size_t len) { struct mtd_part *part = mtd_to_part(mtd); - return part->parent->_max_bad_blocks(part->parent, - ofs + part->offset, len); + return mtd_max_bad_blocks(part->parent, ofs + part->offset, len); } static inline void free_partition(struct mtd_part *p) @@ -437,59 +429,32 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent, slave->mtd._read = part_read; slave->mtd._write = part_write; - - if (parent->_panic_write) - slave->mtd._panic_write = part_panic_write; - - if (parent->_point && parent->_unpoint) { - slave->mtd._point = part_point; - slave->mtd._unpoint = part_unpoint; - } - - if (parent->_read_oob) - slave->mtd._read_oob = part_read_oob; - if (parent->_write_oob) - slave->mtd._write_oob = part_write_oob; - if (parent->_read_user_prot_reg) - slave->mtd._read_user_prot_reg = part_read_user_prot_reg; - if (parent->_read_fact_prot_reg) - slave->mtd._read_fact_prot_reg = part_read_fact_prot_reg; - if (parent->_write_user_prot_reg) - slave->mtd._write_user_prot_reg = part_write_user_prot_reg; - if (parent->_lock_user_prot_reg) - slave->mtd._lock_user_prot_reg = part_lock_user_prot_reg; - if (parent->_get_user_prot_info) - slave->mtd._get_user_prot_info = part_get_user_prot_info; - if (parent->_get_fact_prot_info) - slave->mtd._get_fact_prot_info = part_get_fact_prot_info; - if (parent->_sync) - slave->mtd._sync = part_sync; - if (!partno && !parent->dev.class && parent->_suspend && - parent->_resume) { + slave->mtd._panic_write = part_panic_write; + slave->mtd._point = part_point; + slave->mtd._unpoint = part_unpoint; + slave->mtd._read_oob = part_read_oob; + slave->mtd._write_oob = part_write_oob; + slave->mtd._read_user_prot_reg = part_read_user_prot_reg; + slave->mtd._read_fact_prot_reg = part_read_fact_prot_reg; + slave->mtd._write_user_prot_reg = part_write_user_prot_reg; + slave->mtd._lock_user_prot_reg = part_lock_user_prot_reg; + slave->mtd._get_user_prot_info = part_get_user_prot_info; + slave->mtd._get_fact_prot_info = part_get_fact_prot_info; + slave->mtd._sync = part_sync; + if (!partno && !parent->dev.class) { slave->mtd._suspend = part_suspend; slave->mtd._resume = part_resume; } - if (parent->_writev) - slave->mtd._writev = part_writev; - if (parent->_lock) - slave->mtd._lock = part_lock; - if (parent->_unlock) - slave->mtd._unlock = part_unlock; - if (parent->_is_locked) - slave->mtd._is_locked = part_is_locked; - if (parent->_block_isreserved) - slave->mtd._block_isreserved = part_block_isreserved; - if (parent->_block_isbad) - slave->mtd._block_isbad = part_block_isbad; - if (parent->_block_markbad) - slave->mtd._block_markbad = part_block_markbad; - if (parent->_max_bad_blocks) - slave->mtd._max_bad_blocks = part_max_bad_blocks; - - if (parent->_get_device) - slave->mtd._get_device = part_get_device; - if (parent->_put_device) - slave->mtd._put_device = part_put_device; + slave->mtd._writev = part_writev; + slave->mtd._lock = part_lock; + slave->mtd._unlock = part_unlock; + slave->mtd._is_locked = part_is_locked; + slave->mtd._block_isreserved = part_block_isreserved; + slave->mtd._block_isbad = part_block_isbad; + slave->mtd._block_markbad = part_block_markbad; + slave->mtd._max_bad_blocks = part_max_bad_blocks; + slave->mtd._get_device = part_get_device; + slave->mtd._put_device = part_put_device; slave->mtd._erase = part_erase; slave->parent = parent;