Differential D59126

tty: Revalidate after dropping the tty lock in ioctl handlers
ClosedPublic
Actions

Authored by markj on Sun, Aug 23, 2:30 PM.

Tags

None

Referenced Files

	F168413039: D59126.diff
	Fri, Aug 28, 3:44 AM

	F168324282: D59126.id185040.diff
	Thu, Aug 27, 3:18 PM

	F168317559: D59126.id184790.diff
	Thu, Aug 27, 2:40 PM

	F168310651: D59126.diff
	Thu, Aug 27, 2:01 PM

	Unknown Object (File)
	Wed, Aug 26, 9:41 PM

	Unknown Object (File)
	Wed, Aug 26, 11:46 AM

	Unknown Object (File)
	Wed, Aug 26, 10:48 AM

	Unknown Object (File)
	Wed, Aug 26, 7:30 AM

View All 11 Files

Subscribers

Details

Reviewers

kib
kevans

Group Reviewers

Commits

rGd7883b84b1e1: tty: Revalidate after dropping the tty lock in ioctl handlers
rGb936c3417027: tty: Revalidate after dropping the tty lock in ioctl handlers
rG1aefa1fab9b5: tty: Revalidate after dropping the tty lock in ioctl handlers
rG45a190c827e4: tty: Revalidate after dropping the tty lock in ioctl handlers
rG6269b9bce0e3: tty: Revalidate after dropping the tty lock in ioctl handlers
rGc7cec6fa6e4a: tty: Revalidate after dropping the tty lock in ioctl handlers
rGb207f754c770: tty: Revalidate after dropping the tty lock in ioctl handlers

Summary

The TIOCSCTTY ioctl handler drops the tty lock in order to acquire the
proctree relock. After relocking the tty, it did not revalidate the
tty state, and it could end up linking a doomed tty to the calling
process' session. This race can be exploited to escalate privileges.

TIOCSPGRP has a similar race, fix that too.

Reported by: tsune of GMO Cybersecurity by Ierae, Inc. working with TrendAI Zero Day Initiative

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

markj created this revision.Sun, Aug 23, 2:30 PM

markj held this revision as a draft.

Herald added subscribers: olce, imp. · View Herald TranscriptSun, Aug 23, 2:30 PM

Harbormaster completed remote builds in B75981: Diff 184790.Sun, Aug 23, 2:30 PM

markj published this revision for review.Sun, Aug 23, 2:31 PM

markj changed the visibility from "Public (No Login Required)" to "Subscribers".

markj changed the edit policy from "All Users" to "Subscribers".

markj added reviewers: kib, kevans, secteam.

markj removed subscribers: imp, olce.

markj added subscribers: kib, kevans, secteam.

This is fine, but I think that we can avoid the trouble by locking the proctree_lock around the ioctl handler instead. Please see D59132

markj abandoned this revision.Sun, Aug 23, 10:52 PM

I believe this is fine for the next batch, my patch requires more work.
[Cannot accept due to the review state]

markj reclaimed this revision.Mon, Aug 24, 2:25 PM

kib accepted this revision.Mon, Aug 24, 2:27 PM

This revision is now accepted and ready to land.Mon, Aug 24, 2:27 PM

Closed by commit rGb207f754c770: tty: Revalidate after dropping the tty lock in ioctl handlers (authored by markj). · Explain WhyTue, Aug 25, 4:00 PM

This revision was automatically updated to reflect the committed changes.

markj added a commit: rGb207f754c770: tty: Revalidate after dropping the tty lock in ioctl handlers.

markj added a commit: rGc7cec6fa6e4a: tty: Revalidate after dropping the tty lock in ioctl handlers.

markj added a commit: rG6269b9bce0e3: tty: Revalidate after dropping the tty lock in ioctl handlers.

markj added a commit: rG45a190c827e4: tty: Revalidate after dropping the tty lock in ioctl handlers.

markj added a commit: rG1aefa1fab9b5: tty: Revalidate after dropping the tty lock in ioctl handlers.

markj added a commit: rGb936c3417027: tty: Revalidate after dropping the tty lock in ioctl handlers.Tue, Aug 25, 4:02 PM

markj added a commit: rGd7883b84b1e1: tty: Revalidate after dropping the tty lock in ioctl handlers.Tue, Aug 25, 4:07 PM

markj changed the visibility from "Subscribers" to "Public (No Login Required)".Tue, Aug 25, 5:59 PM

markj changed the edit policy from "Subscribers" to "All Users".

Revision Contents
Changeset List

Path

Size

sys/

kern/

14 lines

Diff 185040

sys/kern/tty.c

Loading...