This bug is marked as fixed by commit:
ext4: block range must be validated before use in ext4_mb_clear_bb()
But I can't find it in any tested tree for more than 90 days.
Is it a correct commit? Please update it by replying:
#syz fix: exact-commit-title
Until then the bug is still considered open and
new crashes with the same signature are ignored.
On Thu, Dec 15, 2022 at 08:34:35AM -0800, syzbot wrote:
> This bug is marked as fixed by commit:
> ext4: block range must be validated before use in ext4_mb_clear_bb()
> But I can't find it in any tested tree for more than 90 days.
> Is it a correct commit? Please update it by replying:
> #syz fix: exact-commit-title
> Until then the bug is still considered open and
> new crashes with the same signature are ignored.
I don't know what is going on with syzkaller's commit detection, but
commit 1e1c2b86ef86 ("ext4: block range must be validated before use
in ext4_mb_clear_bb()") is an exact match for the commit title, and
it's been in the upstream kernel since v6.0.
How do we make syzkaller accept this? I'll try this again, but I
don't hold out much hope.
#syz fix: ext4: block range must be validated before use in ext4_mb_clear_bb()
Syzkaller, go home, you're drunk.
- Ted
On Thu, 15 Dec 2022, Theodore Ts'o wrote:
> On Thu, Dec 15, 2022 at 08:34:35AM -0800, syzbot wrote:
> > This bug is marked as fixed by commit:
> > ext4: block range must be validated before use in ext4_mb_clear_bb()
> > But I can't find it in any tested tree for more than 90 days.
> > Is it a correct commit? Please update it by replying:
> > #syz fix: exact-commit-title
> > Until then the bug is still considered open and
> > new crashes with the same signature are ignored.
>
> I don't know what is going on with syzkaller's commit detection, but
> commit 1e1c2b86ef86 ("ext4: block range must be validated before use
> in ext4_mb_clear_bb()") is an exact match for the commit title, and
> it's been in the upstream kernel since v6.0.
>
> How do we make syzkaller accept this? I'll try this again, but I
> don't hold out much hope.
I don't see the original bug report (was it posted to a lore
associated list?), so there is no way to tell what branch syzbot was
fuzzing at the time. My assumption is that it was !Mainline.
Although this does appear to be a Stable candidate, I do not see it
in any of the Stable branches yet. So I suspect the answer here is to
wait for the fix to filter down.
In the mean time, I guess we should discuss whether syzbot should
really be posting scans of downstream trees to upstream lists.
> #syz fix: ext4: block range must be validated before use in ext4_mb_clear_bb()
>
> Syzkaller, go home, you're drunk.
=:-)
--
Lee Jones [李琼斯]
On Fri, Dec 16, 2022 at 2:01 PM Lee Jones <[email protected]> wrote:
>
> On Thu, 15 Dec 2022, Theodore Ts'o wrote:
>
> > On Thu, Dec 15, 2022 at 08:34:35AM -0800, syzbot wrote:
> > > This bug is marked as fixed by commit:
> > > ext4: block range must be validated before use in ext4_mb_clear_bb()
> > > But I can't find it in any tested tree for more than 90 days.
> > > Is it a correct commit? Please update it by replying:
> > > #syz fix: exact-commit-title
> > > Until then the bug is still considered open and
> > > new crashes with the same signature are ignored.
> >
> > I don't know what is going on with syzkaller's commit detection, but
> > commit 1e1c2b86ef86 ("ext4: block range must be validated before use
> > in ext4_mb_clear_bb()") is an exact match for the commit title, and
> > it's been in the upstream kernel since v6.0.
> >
> > How do we make syzkaller accept this? I'll try this again, but I
> > don't hold out much hope.
>
> I don't see the original bug report (was it posted to a lore
> associated list?), so there is no way to tell what branch syzbot was
> fuzzing at the time. My assumption is that it was !Mainline.
Syzbot is actually reacting here to this bug from the Android namespace:
https://syzkaller.appspot.com/bug?id=5266d464285a03cee9dbfda7d2452a72c3c2ae7c
>
> Although this does appear to be a Stable candidate, I do not see it
> in any of the Stable branches yet. So I suspect the answer here is to
> wait for the fix to filter down.
>
> In the mean time, I guess we should discuss whether syzbot should
> really be posting scans of downstream trees to upstream lists.
In this particular case, syzbot has captured all the recipients from
the patch email [1], because that email Cc'd
[email protected]. To syzbot, all
these people were involved in the original bug discussion, and so it
notified them about the problem.
FWIW I've sent a PR[2] to make the "I can't find it in any tested
tree" message include the link to the syzkaller dashboard. Hopefully
it will help resolve such confusions faster.
[1] https://lore.kernel.org/all/[email protected]/
[2] https://github.com/google/syzkaller/pull/3591
--
Aleksandr
>
> > #syz fix: ext4: block range must be validated before use in ext4_mb_clear_bb()
> >
> > Syzkaller, go home, you're drunk.
>
> =:-)
>
> --
> Lee Jones [李琼斯]
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-android-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-android-bugs/Y5xsIkpIznpObOJL%40google.com.
On Fri, 16 Dec 2022, Aleksandr Nogikh wrote:
> On Fri, Dec 16, 2022 at 2:01 PM Lee Jones <[email protected]> wrote:
> >
> > On Thu, 15 Dec 2022, Theodore Ts'o wrote:
> >
> > > On Thu, Dec 15, 2022 at 08:34:35AM -0800, syzbot wrote:
> > > > This bug is marked as fixed by commit:
> > > > ext4: block range must be validated before use in ext4_mb_clear_bb()
> > > > But I can't find it in any tested tree for more than 90 days.
> > > > Is it a correct commit? Please update it by replying:
> > > > #syz fix: exact-commit-title
> > > > Until then the bug is still considered open and
> > > > new crashes with the same signature are ignored.
> > >
> > > I don't know what is going on with syzkaller's commit detection, but
> > > commit 1e1c2b86ef86 ("ext4: block range must be validated before use
> > > in ext4_mb_clear_bb()") is an exact match for the commit title, and
> > > it's been in the upstream kernel since v6.0.
> > >
> > > How do we make syzkaller accept this? I'll try this again, but I
> > > don't hold out much hope.
> >
> > I don't see the original bug report (was it posted to a lore
> > associated list?), so there is no way to tell what branch syzbot was
> > fuzzing at the time. My assumption is that it was !Mainline.
>
> Syzbot is actually reacting here to this bug from the Android namespace:
>
> https://syzkaller.appspot.com/bug?id=5266d464285a03cee9dbfda7d2452a72c3c2ae7c
>
> > Although this does appear to be a Stable candidate, I do not see it
> > in any of the Stable branches yet. So I suspect the answer here is to
> > wait for the fix to filter down.
> >
> > In the mean time, I guess we should discuss whether syzbot should
> > really be posting scans of downstream trees to upstream lists.
>
> In this particular case, syzbot has captured all the recipients from
> the patch email [1], because that email Cc'd
> [email protected]. To syzbot, all
> these people were involved in the original bug discussion, and so it
> notified them about the problem.
>
> FWIW I've sent a PR[2] to make the "I can't find it in any tested
> tree" message include the link to the syzkaller dashboard. Hopefully
> it will help resolve such confusions faster.
That's helpful, thank you.
--
Lee Jones [李琼斯]
On Fri, Dec 16, 2022 at 03:09:04PM +0100, Aleksandr Nogikh wrote:
>
> Syzbot is actually reacting here to this bug from the Android namespace:
>
> https://syzkaller.appspot.com/bug?id=5266d464285a03cee9dbfda7d2452a72c3c2ae7c
Thanks for the clarification; stupid question, though -- I see
"upstream" is listed on the dashboard link above. Assuming that
"usptream" is "Linus's tree", why was it still saying, "I can't find
this patch in any of my trees"? What about the upstream tree?
> > Although this does appear to be a Stable candidate, I do not see it
> > in any of the Stable branches yet. So I suspect the answer here is to
> > wait for the fix to filter down.
The reason why it's not hit any of the long-term stable trees is
because the patch doesn't apply cleanly, because there are
pre-requisite commits that were required. Here are the required
commits for 5.15:
https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git ext4_for_5.15.83
% git log --reverse --oneline v5.15.83..
96d070a12a7c ext4: refactor ext4_free_blocks() to pull out ext4_mb_clear_bb()
[ Upstream commit 8ac3939db99f99667b8eb670cf4baf292896e72d ]
2fa7a1780ecd ext4: add ext4_sb_block_valid() refactored out of ext4_inode_block_valid()
[ Upstream commit 6bc6c2bdf1baca6522b8d9ba976257d722423085 ]
8dc76aa246b1 ext4: add strict range checks while freeing blocks
[ Upstream commit a00b482b82fb098956a5bed22bd7873e56f152f1 ]
deb2e1554497 ext4: block range must be validated before use in ext4_mb_clear_bb()
[ Upstream commit 1e1c2b86ef86a8477fd9b9a4f48a6bfe235606f6 ]
Further backports to LTS kernels for 5.10, 5.4, etc., are left as an
exercise to the reader. :-)
- Ted
P.S. I have not tried to run gce-xfstests regressions yet. so the
only QA done on these backports is "it builds, ship it!" (And it
fixes the syzbot reproducers.) Then again, we're not running this
kind of regression tests on the LTS kernels.
P.P.S. If anyone is willing to volunteer to be an ext4 backports
maintainer, please contact me. The job description is (a) dealing
with the stable backport failures and addressing the patch conflicts,
potentially by dragging in patch prerequisites, and (b) running
"gce-xfstests ltm -c ext4/all -g auto" and making sure there are no
regressions.
- Ted
On Fri, Dec 16, 2022 at 6:05 PM Theodore Ts'o <[email protected]> wrote:
>
> On Fri, Dec 16, 2022 at 03:09:04PM +0100, Aleksandr Nogikh wrote:
> >
> > Syzbot is actually reacting here to this bug from the Android namespace:
> >
> > https://syzkaller.appspot.com/bug?id=5266d464285a03cee9dbfda7d2452a72c3c2ae7c
>
> Thanks for the clarification; stupid question, though -- I see
> "upstream" is listed on the dashboard link above. Assuming that
> "usptream" is "Linus's tree", why was it still saying, "I can't find
> this patch in any of my trees"? What about the upstream tree?
Bugs from different namespaces are treated independently, so in this
particular case syzbot was expecting the fixing commit to reach the
Android trees that it fuzzes.
--
Aleksandr
>
> > > Although this does appear to be a Stable candidate, I do not see it
> > > in any of the Stable branches yet. So I suspect the answer here is to
> > > wait for the fix to filter down.
>
> The reason why it's not hit any of the long-term stable trees is
> because the patch doesn't apply cleanly, because there are
> pre-requisite commits that were required. Here are the required
> commits for 5.15:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git ext4_for_5.15.83
>
> % git log --reverse --oneline v5.15.83..
> 96d070a12a7c ext4: refactor ext4_free_blocks() to pull out ext4_mb_clear_bb()
> [ Upstream commit 8ac3939db99f99667b8eb670cf4baf292896e72d ]
> 2fa7a1780ecd ext4: add ext4_sb_block_valid() refactored out of ext4_inode_block_valid()
> [ Upstream commit 6bc6c2bdf1baca6522b8d9ba976257d722423085 ]
> 8dc76aa246b1 ext4: add strict range checks while freeing blocks
> [ Upstream commit a00b482b82fb098956a5bed22bd7873e56f152f1 ]
> deb2e1554497 ext4: block range must be validated before use in ext4_mb_clear_bb()
> [ Upstream commit 1e1c2b86ef86a8477fd9b9a4f48a6bfe235606f6 ]
>
> Further backports to LTS kernels for 5.10, 5.4, etc., are left as an
> exercise to the reader. :-)
>
> - Ted
>
> P.S. I have not tried to run gce-xfstests regressions yet. so the
> only QA done on these backports is "it builds, ship it!" (And it
> fixes the syzbot reproducers.) Then again, we're not running this
> kind of regression tests on the LTS kernels.
>
> P.P.S. If anyone is willing to volunteer to be an ext4 backports
> maintainer, please contact me. The job description is (a) dealing
> with the stable backport failures and addressing the patch conflicts,
> potentially by dragging in patch prerequisites, and (b) running
> "gce-xfstests ltm -c ext4/all -g auto" and making sure there are no
> regressions.
>
> - Ted
On Fri, Dec 16, 2022 at 06:14:50PM +0100, Aleksandr Nogikh wrote:
> > Thanks for the clarification; stupid question, though -- I see
> > "upstream" is listed on the dashboard link above. Assuming that
> > "usptream" is "Linus's tree", why was it still saying, "I can't find
> > this patch in any of my trees"? What about the upstream tree?
>
> Bugs from different namespaces are treated independently, so in this
> particular case syzbot was expecting the fixing commit to reach the
> Android trees that it fuzzes.
Is there a way someone can look at the dashboard link to determine
which (a) what namespace a particular syzkaller report is in, and (b)
what trees are included in a particular namespace?
Adding a link to the e-mail to the dashboard page may not help if it's
not obvious why the dashboard mentions "upstream" and yet it's not in
"any of the trees". Maybe the e-mail should explicitly list the trees
that syzkaller will be searching?
And it would seem that it would be a *feature* if looking at a syzbot
dashboard from Android namespace could expose the fact that particular
patch is in any of the LTS trees or Linus's upstream tree, no?
Also, what is the reason for Android for being in a separate
namespace? Is it running on a separate syzbot VM? I can understand
why from a feature perspective, that Fuschia and OpenBSD should be in
separate namespaces; but what are the reasons that there are separate
namespaces for Android versus the upstream kernel? Especially since
the Android dashboard is apparently referencing the upstream kernel?
What's up with that?
Put another way, while I think it's super useful to have a link to
Syzbot dashboard page, in the e-mail, I'm not sure it's going to be a
complete solution to the confusion that was inspired by this case.
That being said, in general I think a link to the Dashboard is useful;
in fact, it might be nice if we could encourage upstream developers
put in the commit trailer:
Link: https://syzkaller.appspot.com/bug?id=5266d464285a03cee9dbfda7d2452a72c3c2ae7c
in addition to, or better yet, instead of:
Reported-by: [email protected]
... and have Syzbot be able to translate from the Link: tag as being
equivalent to the Reported-by: link. That's becase the Link is going
to be much more useful to humans than the Reported-by --- we've had a
number of cases where as part of the patch review, we really wanted to
get back to the Dashboard page, and it's not easy to get to the
Dashboard from the Reported-by tag.
Thanks,
- Ted
Hi Ted,
Thanks for the comments!
On Fri, Dec 16, 2022 at 7:45 PM Theodore Ts'o <[email protected]> wrote:
>
> On Fri, Dec 16, 2022 at 06:14:50PM +0100, Aleksandr Nogikh wrote:
> > > Thanks for the clarification; stupid question, though -- I see
> > > "upstream" is listed on the dashboard link above. Assuming that
> > > "usptream" is "Linus's tree", why was it still saying, "I can't find
> > > this patch in any of my trees"? What about the upstream tree?
> >
> > Bugs from different namespaces are treated independently, so in this
> > particular case syzbot was expecting the fixing commit to reach the
> > Android trees that it fuzzes.
>
> Is there a way someone can look at the dashboard link to determine
> which (a) what namespace a particular syzkaller report is in, and (b)
> what trees are included in a particular namespace?
(a) Once you have opened the bug report page, you can find the
namespace at the top of the page.
(b) One can at least see the list of the tested trees on the main page
of the namespace -- we do share the latest commits for each manager
instance. Also see the comment below.
>
> Adding a link to the e-mail to the dashboard page may not help if it's
> not obvious why the dashboard mentions "upstream" and yet it's not in
> "any of the trees". Maybe the e-mail should explicitly list the trees
> that syzkaller will be searching?
I've sent a PR[1] that makes the bot send the list of the searched
trees. For upstream, we search quite a lot of trees, so the bot will
share some of them in the email and give a link to see the rest.
Otherwise the contents would be totally unintelligible.
[1] https://github.com/google/syzkaller/pull/3593
>
> And it would seem that it would be a *feature* if looking at a syzbot
> dashboard from Android namespace could expose the fact that particular
> patch is in any of the LTS trees or Linus's upstream tree, no?
Yes, that would be definitely nice.
We do have the improvements to the missing commit detection on our
TODO list, but I cannot say at the moment when exactly it will be
done.
>
> Also, what is the reason for Android for being in a separate
> namespace? Is it running on a separate syzbot VM? I can understand
> why from a feature perspective, that Fuschia and OpenBSD should be in
> separate namespaces; but what are the reasons that there are separate
> namespaces for Android versus the upstream kernel? Especially since
> the Android dashboard is apparently referencing the upstream kernel?
> What's up with that?
It's based on Linux, but it's not exactly Linux and can have its own bugs.
>
> Put another way, while I think it's super useful to have a link to
> Syzbot dashboard page, in the e-mail, I'm not sure it's going to be a
> complete solution to the confusion that was inspired by this case.
>
> That being said, in general I think a link to the Dashboard is useful;
> in fact, it might be nice if we could encourage upstream developers
> put in the commit trailer:
>
> Link: https://syzkaller.appspot.com/bug?id=5266d464285a03cee9dbfda7d2452a72c3c2ae7c
>
> in addition to, or better yet, instead of:
>
> Reported-by: [email protected]
>
> ... and have Syzbot be able to translate from the Link: tag as being
> equivalent to the Reported-by: link. That's becase the Link is going
> to be much more useful to humans than the Reported-by --- we've had a
> number of cases where as part of the patch review, we really wanted to
> get back to the Dashboard page, and it's not easy to get to the
> Dashboard from the Reported-by tag.
FWIW it's quite easy to get the Dashboard link from the Reported-by
tag (although I agree it's not the most intuitive thing imaginable) --
one just needs to substitute the hash code after the + sign to
https://syzkaller.appspot.com/bug?extid=%s
Re. the Link tag.. it's interesting. It doesn't seem to be very
reasonable to include both, as it would look somewhat excessive:
Reported-by: [email protected]
Link: https://syzkaller.appspot.com/bug?extid=abcdef012345678
I'll take a look into the pros and cons of using just the Link tag.
--
Aleksandr
>
> Thanks,
>
> - Ted
>
On Mon, Dec 19, 2022 at 05:12:47PM +0100, Aleksandr Nogikh wrote:
> (a) Once you have opened the bug report page, you can find the
> namespace at the top of the page.
> (b) One can at least see the list of the tested trees on the main page
> of the namespace -- we do share the latest commits for each manager
> instance. Also see the comment below.
It's not obvious what you mean by the "main page" of the namespace.
I'm guessing, but from the bug report page, there is a horizontal set
of icons, "Open", "Fixed", "Invalid" .... (which all have the same
icons), that the "Open" icon is the one that gets to the main page?
Assuming that this[1] is what was meant by "main page" (which is also
implied by the URL, but it's otherwise **really** not obvious), where
is the list of tested trees?
[1] https://syzkaller.appspot.com/android-5-10
I see the table "Instances", but it looks like the only two instances,
ci2-android-5-10 and ci2-android-5-10-perf, are both apparently
looking at the same commit --- but there's nothing that tells you what
kernel tree those commits are from. I can't see **anything* that
looks like an explicit git repo URL plus branch name. Is it perhaps
one of these?
https://android.googlesource.com/kernel/common/+/refs/heads/android12-5.10
https://android.googlesource.com/kernel/common/+/refs/heads/android13-5.10
It also appears that the android-5-10 "namespace" doesn't track any
other trees other than the Android 5.10 tree. Which means the e-mail
message, "I can't find the commit on any tested tree" is ***super***
misleading. At least for the android-5-10 namespace, why not just
say, "I don't see that commit on the git branch <explicit git repo and
branch name>"?
I did finally find the missing information, but it required a lot of
clicking and searching. From the bug page [2], the status line is:
Status: upstream: reported C repro on 2022/11/27 00:51
Has a link to the e-mail sent to the upstream developers[3]. And in
*that* e-mail, we can find the git tree involved:
git tree: android12-5.10-lts
[3] https://groups.google.com/g/syzkaller-android-bugs/c/LmaUwJpTXkA/m/HjsARFKWCQAJ
Going back to your pull request[4] to add a link to the dashboard in
the e-mail, how about also adding to the e-mail an indication about
the Syzkaller namespace. That way, the upstream developer can quickly
determine that the namespace is "Android-X.Y" and simply hit the 'd'
key as not really relevant to the upstream developer.
[4] https://github.com/google/syzkaller/pull/3591
I assume that there's someone in the Android kernel ecosystem that is
responsible for figuring out how to make sure commits are backported
from upstream into the LTS kernels, and the LTS kernels to the
relevant Android branch.
I do know one thing for certain --- I don't scale to the point where
this can made my problem. So I want to be able to more quickly triage
e-mails that are Not My Problem.
More generally, I think we need some kind of MAINTAINERS-like file
which explicitly lists who does have that responsibility, and which
can be used by Syzkaller so we aren't just blindly spamming all of the
upstream developers in the hopes that one of them will do somebody
else's job just to shut up the nag mail. Otherwise, the natural
reaction will be to resort to a mail filter to /dev/null the nag mail.
:-/
- Ted