| Message ID | 20210810091800.291272-1-islituo@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: core: Fix possible null-pointer dereference in failover_slave_register() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 4 of 4 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | CHECK: Alignment should match open parenthesis WARNING: line length of 85 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Tue, 10 Aug 2021 02:18:00 -0700 Tuo Li wrote: > The variable fops is checked in: > if (fops && fops->slave_pre_register && > fops->slave_pre_register(slave_dev, failover_dev)) > > This indicates that it can be NULL. > However, it is dereferenced when calling netdev_rx_handler_register(): > err = netdev_rx_handler_register(slave_dev, fops->slave_handle_frame, > failover_dev); > > To fix this possible null-pointer dereference, check fops first, and if > it is NULL, assign -EINVAL to err. The other fops checks look like defensive programming. I don't see anywhere where fops would be cleared, and all callers pass it to register().
diff --git a/net/core/failover.c b/net/core/failover.c index b5cd3c727285..113a4dacdf48 100644 --- a/net/core/failover.c +++ b/net/core/failover.c @@ -63,8 +63,11 @@ static int failover_slave_register(struct net_device *slave_dev) fops->slave_pre_register(slave_dev, failover_dev)) goto done; - err = netdev_rx_handler_register(slave_dev, fops->slave_handle_frame, + if (fops) + err = netdev_rx_handler_register(slave_dev, fops->slave_handle_frame, failover_dev); + else + err = -EINVAL; if (err) { netdev_err(slave_dev, "can not register failover rx handler (err = %d)\n", err);
The variable fops is checked in: if (fops && fops->slave_pre_register && fops->slave_pre_register(slave_dev, failover_dev)) This indicates that it can be NULL. However, it is dereferenced when calling netdev_rx_handler_register(): err = netdev_rx_handler_register(slave_dev, fops->slave_handle_frame, failover_dev); To fix this possible null-pointer dereference, check fops first, and if it is NULL, assign -EINVAL to err. Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> Signed-off-by: Tuo Li <islituo@gmail.com> --- net/core/failover.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)