RE: ubifs: read bad node type in ubifs_tnc_read_wbuf

[李傲傲 (Carson Li1/9542) <Carson.Li1@xxxxxxxxxx>]

Hi,

>It seems the LEB used as DATA journal head is GC'ed, and ubifs_tnc_locate()
>read an invalid node. But now the property of journal head LEB has
>LPROPS_TAKEN flag set and GC will skip these LEBs.

>The actual situation of the problem is the LEB is GCed, freed and then
>reused as journal head, and finally ubifs_tnc_locate() reads an invalid node.

Actually, I think that situation might only be caused by a commit, is that right?
Since only commit might clear the journal head LEBs' property of LPROPS_TAKEN.
But it will not get the c->jheads[i].wbuf->lnum LEB's taken property cleared, so there
seems no need to check if a c->jheads[i].wbuf->lnum LEB might be GCed and the
node whether fully sits in wirte buffer.

but it is another situation if there is not only one but two or more commits happen,
the LEB with jheads[i].wbuf->lnum should be considered whether it has been GCed.
I am not sure if we need to get such a less possible situation into account :( .

>+static int ubifs_check_and_read_wbuf(struct ubifs_info *c,
>+     const struct ubifs_zbranch *zbr,
>+     int gc_seq, void *buf, bool *retry)
>+{
>+bool found = false;
>+int lnum = zbr->lnum;
>+int offs = zbr->offs;
>+int len = zbr->len;
>+int type;
>+int i;
>+int err;
>+
>+*retry = false;
>+for (i = 0; i < c->jhead_cnt; i++) {
>+struct ubifs_wbuf *wbuf = &c->jheads[i].wbuf;
>+
>+/* Check whether the node is fully included in wbuf */
>+spin_lock(&wbuf->lock);
>+if (wbuf->lnum == lnum && wbuf->offs <= offs &&
>+    offs + len <= wbuf->offs + wbuf->used) {
>+/*
>+ * lnum is GC'ed and reused as journal head,
>+ * we need to lookup TNC again.
>+ */
>+if (maybe_leb_gced(c, lnum, gc_seq)) {
>+spin_unlock(&wbuf->lock);
>+*retry = true;
>+break;
>+}
>+
>+memcpy(buf, wbuf->buf + offs - wbuf->offs, len);
>+spin_unlock(&wbuf->lock);
>+found = true;
>+break;
>+}
>+spin_unlock(&wbuf->lock);
>+}
>+

Is it more safely to have may_leb_gced after memcpy though memcpy is quickly?

>+if (!found)
+return 0;
+
+type = key_type(c, &zbr->key);
+err = ubifs_check_node_buf(c, buf, type, len, lnum, offs);
+if (err)
+return err;
+
+return 1;
+}
+

I don't find the ubifs_check_node_buf , is it a function you newly defined in this patch?

Thanks.
Carson

________________________________
 This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/