I removed the call to setpflags(), and the target failed to come back online for about 1 minute after being offlined.
-jim Peter Dunlap wrote: > James Carlson wrote: > >> Peter Dunlap writes: >> >> >>> James Carlson wrote: >>> >>> >>>> 957-966: this doesn't look right to me. I strongly suggest that you >>>> contact the TX team ([EMAIL PROTECTED] or >>>> [EMAIL PROTECTED]) to discuss the meaning of this >>>> flag. Only MAC-aware applications ought to be setting this, and I >>>> don't see where this code handles mandatory access controls at all. >>>> >>>> >>>> >>> It's not as bad as it looks -- we are giving up the privilege rather >>> than acquiring it. >>> >>> >> Giving it up for whom? This is blowing away the NET_MAC_AWARE flag on >> the proc_t associated with the current CRED(). I don't know what >> thread is running here, and thus I don't know what cred_t you're >> actually using (and thus what proc_t gets hit). If it's a kernel >> thread, then you're disabling the mac-aware processing for the kernel >> itself. If it's a user thread, then you're removing mac-aware >> processing for some arbitrary user process, and arbitrarily turning >> off a special privilege that the process once held. >> >> I don't see how manipulating the process flags or privileges from >> within a driver (particularly something as common and invisible to >> applications as a block storage driver) is a useful thing to do. It >> might possibly make some sense if you had your own kernel thread or >> special daemon process and were manipulating the flags on that, though >> that might beg other questions, but doing it on a thread that belongs >> to someone else just can't be right. Can it? >> >> This almost certainly breaks TX. >> >> What originally caused me to write a code review comment here was the >> phrase "[t]here seem to be some security ramifications." Seems, sir? >> Nay, it is; I know not "seems." >> >> > > It seemed wrong to take the workaround from the (already code reviewed) > ON component without taking the comment with it. > > Look, this is the exact reason I explicitly sought feedback from > networking. It sounds like this is something we need to dig into and > understand further. We will remove the workaround and re-test with the > current ON bits. Maybe this was a problem with an older version of ON. > If it behaves as it did before (badly) we will file a bug and solicit > help from the networking team. Is that acceptable? > > >> (Which is another way of saying "you need at least a design level >> comment here; if not something more significant." Thanks to Sowmini >> for the Hamlet reference reminder. ;-}) >> >> >> >>> CIFS server actually does the same thing: >>> >>> >> As I noted in other cases, I don't really care who or what already >> does things like this. >> >> >> > > I think I made it clear in a separate e-mail that we are committed to > resolving your review comments. My mention of the CIFS server is not > intended to be an excuse nor am I waving it around like a free pass. My > point is that if this change breaks something then that something is > *already* broken. So we should probably file a fairly high priority bug > against CIFS server right? > > >> >> >>> My understanding when I originally came up with this workaround (for the >>> CIFS server code) was that since we were using a kernel thread we got >>> the NET_MAC_AWARE privilege whether we wanted it or not. Typically a >>> socket thread would come from user space and would not have this >>> problem. The only components using sockfs from the kernel are iSCSI >>> initiator (which doesn't call soaccept), CIFS server (which uses this >>> workaround) and now our code. This is the sort of thing that makes me >>> eagerly anticipate the volo project. >>> >>> I will solicit some input from rampart-dev-team. With the explanation >>> above do you think its busted or is it just fishy? >>> >>> >> It looks busted to me. >> >> >> > > Thanks, that's what I was looking for. > > -Peter > > _______________________________________________ > iser-dev mailing list > [EMAIL PROTECTED] > http://mail.opensolaris.org/mailman/listinfo/iser-dev > _______________________________________________ networking-discuss mailing list [email protected]
