audit_remove_watch_rule() drops watch's reference to parent but then continues to work with it. That is not safe as parent can get freed once we drop our reference. The following is a trivial reproducer: mount -o loop image /mnt touch /mnt/file auditctl -w /mnt/file -p wax umount /mnt auditctl -D <crash in fsnotify_destroy_mark()> Grab our own reference in audit_remove_watch_rule() earlier to make sure mark does not get freed under us.
Unified · Split
| @@ -457,13 +457,15 @@ void audit_remove_watch_rule(struct audit_krule *krule) · 456 unmodified lines | |||
| 457 | list_del(&krule->rlist); | 457 | list_del(&krule->rlist); |
| 458 | 458 | ||
| 459 | if (list_empty(&watch->rules)) { | 459 | if (list_empty(&watch->rules)) { |
| 460 | /* | ||
| 461 | * audit_remove_watch() drops our reference to 'parent' which | ||
| 462 | * can get freed. Grab our own reference to be safe. | ||
| 463 | */ | ||
| 464 | audit_get_parent(parent); | ||
| 460 | audit_remove_watch(watch); | 465 | audit_remove_watch(watch); |
| 461 | 466 | if (list_empty(&parent->watches)) | |
| 462 | if (list_empty(&parent->watches)) { | ||
| 463 | audit_get_parent(parent); | ||
| 464 | fsnotify_destroy_mark(&parent->mark, audit_watch_group); | 467 | fsnotify_destroy_mark(&parent->mark, audit_watch_group); |
| 465 | audit_put_parent(parent); | 468 | audit_put_parent(parent); |
| 466 | } | ||
| 467 | } | 469 | } |
| 468 | } | 470 | } |
| 469 | 471 | ||