| Message ID | 20250129085017.55991-1-asharji1828@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v3] net: ipmr: Fix out-of-bounds access i mr_mfc_uses_dev() | expand |
On Wed, 29 Jan 2025 12:50:17 +0400 Abdullah wrote: > The issue was reported by Syzbot as an out-of-bounds read: > UBSAN: array-index-out-of-bounds in net/ipv4/ipmr_base.c:289:10 > Index -772737152 is out of range for type 'const struct vif_device[32]' > > The problem occurs when the minvif/maxvif values in the mr_mfc struct > become invalid (possibly due to memory corruption or uninitialized values). > This patch fixes the issue by ensuring proper boundary checks and rcu_read > locking before accessing vif_table[] in mr_mfc_uses_dev(). > > Fixes: <COMMIT_HASH> > Reported-by: syzbot+5cfae50c0e5f2c500013@syzkaller.appspotmail.com > Signed-off-by: Abdullah <asharji1828@gmail.com> Could you explain what you're trying to do here? Are you just tossing patches to test at syzbot? If yes, please remove the unnecessary CCs, reply directly to the syzbot address, there is no need to spam the mailing lists. Or do you mean this as a real submissions? In which case why is there <COMMIT_HASH> instead of the correct commit? The entire submission feels a little.. LLM-aided.
diff --git a/net/ipv4/ipmr_base.c b/net/ipv4/ipmr_base.c index 03b6eee407a2..7c38d0cf41fc 100644 --- a/net/ipv4/ipmr_base.c +++ b/net/ipv4/ipmr_base.c @@ -280,9 +280,31 @@ static bool mr_mfc_uses_dev(const struct mr_table *mrt, const struct mr_mfc *c, const struct net_device *dev) { + /** + * Helper function that checks if *dev is part of the OIL (Outgoing Interfaces List). + * @mrt: Is the multi-routing table. + * @c: Is the Multicast Forwarding Cache. + * @dev: The net device being checked. + * + * vif_dev: Pointer to the net device's struct. + * vif: Pointer to the actual device. + * + * OIL is a subset of mrt->vif_table[]. + * minvif: Start index of OIL in vif_table[]. + * maxvif: End index of OIL in vif_table[]. + * + * Returns: + * - true if `dev` is part of the OIL. + * - false otherwise. + */ + int ct; + + int minvif = c->mfc_un.res.minvif, maxvif = c->mfc_un.res.maxvif; + if (minvif < 0 || maxvif > 32) + return false; - for (ct = c->mfc_un.res.minvif; ct < c->mfc_un.res.maxvif; ct++) { + for (ct = minvif; ct < maxvif; ct++) { const struct net_device *vif_dev; const struct vif_device *vif; @@ -309,7 +331,8 @@ int mr_table_dump(struct mr_table *mrt, struct sk_buff *skb, if (filter->filter_set) flags |= NLM_F_DUMP_FILTERED; - + + rcu_read_lock(); list_for_each_entry_rcu(mfc, &mrt->mfc_cache_list, list, lockdep_rtnl_is_held()) { if (e < s_e) @@ -325,7 +348,8 @@ int mr_table_dump(struct mr_table *mrt, struct sk_buff *skb, next_entry: e++; } - + rcu_read_unlock(); + spin_lock_bh(lock); list_for_each_entry(mfc, &mrt->mfc_unres_queue, list) { if (e < s_e)
The issue was reported by Syzbot as an out-of-bounds read: UBSAN: array-index-out-of-bounds in net/ipv4/ipmr_base.c:289:10 Index -772737152 is out of range for type 'const struct vif_device[32]' The problem occurs when the minvif/maxvif values in the mr_mfc struct become invalid (possibly due to memory corruption or uninitialized values). This patch fixes the issue by ensuring proper boundary checks and rcu_read locking before accessing vif_table[] in mr_mfc_uses_dev(). Fixes: <COMMIT_HASH> Reported-by: syzbot+5cfae50c0e5f2c500013@syzkaller.appspotmail.com Signed-off-by: Abdullah <asharji1828@gmail.com> --- net/ipv4/ipmr_base.c | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-)