Skip to contentSkip to conversation

drbd: potential null dereference

PATCH In review Dan Carpenter 3 participants 4 messages Mar 27, 2010 – Apr 2, 2010

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drbd: potential null dereference

If "e" is NULL then "bm_ext" would be NULL too.

---
Found by static analysis and compile tested only. Sorry. :/

drivers/block/drbd/drbd_actlog.c +1 -1
index 17956ff..b68b778 100644
@@ -1328,7 +1328,7 @@ int drbd_rs_del_all(struct drbd_conf *mdev)
 		for (i = 0; i < mdev->resync->nr_elements; i++) {
 			e = lc_element_by_index(mdev->resync, i);
 			bm_ext = e ? lc_entry(e, struct bm_extent, lce) : NULL;
-			if (bm_ext->lce.lc_number = LC_FREE)
+			if (!bm_ext || bm_ext->lce.lc_number = LC_FREE)
 				continue;
 			if (bm_ext->lce.lc_number = mdev->resync_wenr) {
 				dev_info(DEV, "dropping %u in drbd_rs_del_all, apparently"
maintainer
#1 lore raw

Hi Dan,

Thanks for the work with the static analyzer, but the patch is bogus. The lc_element_by_index() function causes a BUG_ON before returning a NULL. The next mail contains a patch that removes the unnecessary conditional expression operator.

Philipp Reisner (1):
drbd: lc_element_by_index() never returns NULL

 drivers/block/drbd/drbd_actlog.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
maintainer
#2 lore raw

PATCH 1/1 drbd: lc_element_by_index() never returns NULL

---
 drivers/block/drbd/drbd_actlog.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/block/drbd/drbd_actlog.c +1 -1
index 43e57f3..df01899 100644
@@ -1336,7 +1336,7 @@ int drbd_rs_del_all(struct drbd_conf *mdev)
 		/* ok, ->resync is there. */
 		for (i = 0; i < mdev->resync->nr_elements; i++) {
 			e = lc_element_by_index(mdev->resync, i);
-			bm_ext = e ? lc_entry(e, struct bm_extent, lce) : NULL;
+			bm_ext = lc_entry(e, struct bm_extent, lce);
 			if (bm_ext->lce.lc_number == LC_FREE)
 				continue;
 			if (bm_ext->lce.lc_number == mdev->resync_wenr) {
Signature · 1 line
1.6.3.3