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-next>] [day] [month] [year] [list]
Date:   Mon, 2 Oct 2023 14:35:27 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     dregan@...l.com
Cc:     bcm-kernel-feedback-list@...adcom.com,
        linux-mtd@...ts.infradead.org, f.fainelli@...il.com,
        rafal@...ecki.pl, joel.peshkin@...adcom.com,
        computersforpeace@...il.com, dan.beygelman@...adcom.com,
        william.zhang@...adcom.com, frieder.schrempf@...tron.de,
        linux-kernel@...r.kernel.org, vigneshr@...com, richard@....at,
        bbrezillon@...nel.org, kdasu.kdev@...il.com
Subject: Re: [PATCH v2] mtd: rawnand: brcmnand: Initial exec_op
 implementation

Hi David,

dregan@...l.com wrote on Sat, 30 Sep 2023 03:57:35 +0200:

> Initial exec_op implementation for Broadcom STB, Broadband and iProc SoC
> This adds exec_op and removes the legacy interface.
> 
> Signed-off-by: David Regan <dregan@...l.com>
> Reviewed-by: William Zhang <william.zhang@...adcom.com>
> 
> ---
> 

...

> +static int brcmnand_parser_exec_matched_op(struct nand_chip *chip,
> +					 const struct nand_subop *subop)
> +{
> +	struct brcmnand_host *host = nand_get_controller_data(chip);
> +	struct brcmnand_controller *ctrl = host->ctrl;
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	const struct nand_op_instr *instr = &subop->instrs[0];
> +	unsigned int i;
> +	int ret = 0;
> +
> +	for (i = 0; i < subop->ninstrs; i++) {
> +		instr = &subop->instrs[i];
> +
> +		if ((instr->type == NAND_OP_CMD_INSTR) &&
> +			(instr->ctx.cmd.opcode == NAND_CMD_STATUS))
> +			ctrl->status_cmd = 1;
> +		else if (ctrl->status_cmd && (instr->type == NAND_OP_DATA_IN_INSTR)) {
> +			/*
> +			 * need to fake the nand device write protect because nand_base does a
> +			 * nand_check_wp which calls nand_status_op NAND_CMD_STATUS which checks
> +			 * that the nand is not write protected before an operation starts.
> +			 * The problem with this is it's done outside exec_op so the nand is
> +			 * write protected and this check will fail until the write or erase
> +			 * or write back operation actually happens where we turn off wp.
> +			 */
> +			u8 *in;
> +
> +			ctrl->status_cmd = 0;
> +
> +			instr = &subop->instrs[i];
> +			in = instr->ctx.data.buf.in;
> +			in[0] = brcmnand_status(host) | NAND_STATUS_WP; /* hide WP status */

I don't understand why you are faking the WP bit. If it's set,
brcmnand_status() should return it and you should not care about it. If
it's not however, can you please give me the path used when we have
this issue? Either we need to modify the core or we need to provide
additional helpers in this driver to circumvent the faulty path.

> +		} else if (instr->type == NAND_OP_WAITRDY_INSTR) {
> +			ret = bcmnand_ctrl_poll_status(host, NAND_CTRL_RDY, NAND_CTRL_RDY, 0);
> +			if (ctrl->wp_cmd) {
> +				ctrl->wp_cmd = 0;
> +				brcmnand_wp(mtd, 1);

This ideally should disappear.

> +			}
> +		} else { /* otherwise pass to low level implementation */
> +			if ((instr->type == NAND_OP_CMD_INSTR) &&
> +				(instr->ctx.cmd.opcode == NAND_CMD_RESET)) {
> +				brcmnand_status(host);
> +				ctrl->status_cmd = 0;
> +				ctrl->wp_cmd = 0;
> +				brcmnand_wp(mtd, 1);

Same

> +			}
> +
> +			if ((instr->type == NAND_OP_CMD_INSTR) &&
> +				((instr->ctx.cmd.opcode == NAND_CMD_ERASE1) ||
> +				(instr->ctx.cmd.opcode == NAND_CMD_SEQIN))) {
> +				brcmnand_wp(mtd, 0);
> +				ctrl->wp_cmd = 1;

Same

> +			}
> +
> +			ret = brcmnand_exec_instr(host, instr, i == (subop->ninstrs - 1));
> +		}
> +	}
> +
> +	return ret;
> +}



Thanks,
Miquèl

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ