Re: thread-unsafety problems as spl*() ones are NOP

2016-01-31 Thread Slawa Olhovchenkov 
On Sun, Jan 31, 2016 at 10:00:51AM +0330, mokhi wrote:

> @imp:
> i exactly mean (Okay not so exact but very near ;D) what you said.
> after analyzing kbd.c functions (eg, kbd_realloc_array()) i concluded
> there are race conditions (and at  result in some places there are
> un-protected data too)
> 
> i don't mean to blindly replace splXXX() with locks, but the places i
> see race-conds.
> Also i should say there are manythings i dunno well or i dont have
> deep understanding of them and that's why im here to ask (ie what
> special condition Giant-Lock makes here [i should care about] and what
> is MPSAFE basically)
> i'd happy if you answer me those question too :D

I think god explainig can be found in VMS Device Support Manual,
Section 3 (for example
http://odl.sysworks.biz/disk$cddoc04mar21/decw$book/d32va114.p63.decw$book#186).

splXXX is analouge of VMS IPL 1-15 and have same purpose.

Or The Design and Implementation of the 4.3BSD UNIX Operating System
(Chapter 3.3).

Also you can look in FreeBSD repo, stable/2.2 branch, sys/i386/include/spl.h
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread mokhi 
So in your opinion I shouldn't put mutex/spin/lock/unlock under splitty/splx?
Can you please explain a bit more about MPSAFE using for me too?

Regards, Mokhi.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread Hans Petter Selasky 

On 01/30/16 19:31, Slawa Olhovchenkov wrote:

On Sat, Jan 30, 2016 at 09:25:21PM +0300, Slawa Olhovchenkov wrote:


On Sat, Jan 30, 2016 at 09:42:13PM +0330, mokhi wrote:


i currently only wanna do patch on kbd.c (because i'm sure there is a
thread-unsafety)
and i don't want to add anything to spltty() nor splx(), i just wanna
add things under where they've been used.
isn't problem with using mutex/spin/lock/unlock etc there?


yes.
currenly spltty act as mutex_lock(_mutex), like global lock for
all used files. this is also inter-files lock, not only lock inside
only kbd.c.
you need patch all files. all using spltty need to remove
simultaneously.



oh, sorry.
i am see now all splXXX is nop.
may be incorretly -- for example, in digi.c i am don't see any lockind
instead spltty().


Hi,

Code that uses splxxx() needs to have the Giant lock locked when 
running. Refer to the various MPSAFE flags.


--HPS

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread Ian Lepore 
On Sat, 2016-01-30 at 18:56 +0330, mokhi wrote:
> Hi.
> in kbd.c there are many places spltty()/splx() used assuming it
> locks/unlocks.
> though there is bug filed for this, and ive asked in #bsddev, Ive
> preferred to ask and ensure it from here again.
> As these functions are obsoleted now, this assumption is incorrect
> and
> some places we have thread-unsafely which leads to security problems
> (and/or for example double-free, etc)
> 
> can i use mutex/spin/lock/unlock under where assumed a lock/unlock by
> using spltty()/splx() to patch it?
> 
> Thanks, Mokhi.

If you start working on locking in keyboard drivers you might discover
there are dragons there.  For example...

https://lists.freebsd.org/pipermail/svn-src-head/2014-March/056833.html

-- Ian

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread Warner Losh 
On Sat, Jan 30, 2016 at 2:09 PM, Ian Lepore  wrote:

> On Sat, 2016-01-30 at 18:56 +0330, mokhi wrote:
> > Hi.
> > in kbd.c there are many places spltty()/splx() used assuming it
> > locks/unlocks.
> > though there is bug filed for this, and ive asked in #bsddev, Ive
> > preferred to ask and ensure it from here again.
> > As these functions are obsoleted now, this assumption is incorrect
> > and
> > some places we have thread-unsafely which leads to security problems
> > (and/or for example double-free, etc)
> >
> > can i use mutex/spin/lock/unlock under where assumed a lock/unlock by
> > using spltty()/splx() to patch it?
> >
> > Thanks, Mokhi.
>
> If you start working on locking in keyboard drivers you might discover
> there are dragons there.  For example...
>
> https://lists.freebsd.org/pipermail/svn-src-head/2014-March/056833.html


In theory, they are all Giant locked. In reality, however, there are many
dragons, and the dragons are difficult to slay... Though things have
been chipped away enough that it might not be so bad now... I tried early
in the locking game and found too many dependencies on Giant in the
code that was called from the keyboard drivers to be able to make much
progress. But that was in the FreeBSD 6 time frame, and Giant is almost
gone from the rest of the system, so another run might not be so bad.

Warner
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread Slawa Olhovchenkov 
On Sat, Jan 30, 2016 at 09:42:13PM +0330, mokhi wrote:

> i currently only wanna do patch on kbd.c (because i'm sure there is a
> thread-unsafety)
> and i don't want to add anything to spltty() nor splx(), i just wanna
> add things under where they've been used.
> isn't problem with using mutex/spin/lock/unlock etc there?

yes.
currenly spltty act as mutex_lock(_mutex), like global lock for
all used files. this is also inter-files lock, not only lock inside
only kbd.c.
you need patch all files. all using spltty need to remove
simultaneously.

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread mokhi 
Hi.
in kbd.c there are many places spltty()/splx() used assuming it locks/unlocks.
though there is bug filed for this, and ive asked in #bsddev, Ive
preferred to ask and ensure it from here again.
As these functions are obsoleted now, this assumption is incorrect and
some places we have thread-unsafely which leads to security problems
(and/or for example double-free, etc)

can i use mutex/spin/lock/unlock under where assumed a lock/unlock by
using spltty()/splx() to patch it?

Thanks, Mokhi.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread Slawa Olhovchenkov 
On Sat, Jan 30, 2016 at 06:56:00PM +0330, mokhi wrote:

> Hi.
> in kbd.c there are many places spltty()/splx() used assuming it locks/unlocks.
> though there is bug filed for this, and ive asked in #bsddev, Ive
> preferred to ask and ensure it from here again.
> As these functions are obsoleted now, this assumption is incorrect and
> some places we have thread-unsafely which leads to security problems
> (and/or for example double-free, etc)
> 
> can i use mutex/spin/lock/unlock under where assumed a lock/unlock by
> using spltty()/splx() to patch it?

If other parts of kernel sources, locked by spltty()/splx(), don't
interacted by called function and accessed data.

Cuurently, in stable, spltty used in 27 files and splx in 101 files.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread mokhi 
i currently only wanna do patch on kbd.c (because i'm sure there is a
thread-unsafety)
and i don't want to add anything to spltty() nor splx(), i just wanna
add things under where they've been used.
isn't problem with using mutex/spin/lock/unlock etc there?
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread Slawa Olhovchenkov 
On Sat, Jan 30, 2016 at 09:25:21PM +0300, Slawa Olhovchenkov wrote:

> On Sat, Jan 30, 2016 at 09:42:13PM +0330, mokhi wrote:
> 
> > i currently only wanna do patch on kbd.c (because i'm sure there is a
> > thread-unsafety)
> > and i don't want to add anything to spltty() nor splx(), i just wanna
> > add things under where they've been used.
> > isn't problem with using mutex/spin/lock/unlock etc there?
> 
> yes.
> currenly spltty act as mutex_lock(_mutex), like global lock for
> all used files. this is also inter-files lock, not only lock inside
> only kbd.c.
> you need patch all files. all using spltty need to remove
> simultaneously.
> 

oh, sorry.
i am see now all splXXX is nop.
may be incorretly -- for example, in digi.c i am don't see any lockind
instead spltty().
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread mokhi 
@imp So you think I should start to put locks there and see what happens? :)
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread mokhi 
@imp:
i exactly mean (Okay not so exact but very near ;D) what you said.
after analyzing kbd.c functions (eg, kbd_realloc_array()) i concluded
there are race conditions (and at  result in some places there are
un-protected data too)

i don't mean to blindly replace splXXX() with locks, but the places i
see race-conds.
Also i should say there are manythings i dunno well or i dont have
deep understanding of them and that's why im here to ask (ie what
special condition Giant-Lock makes here [i should care about] and what
is MPSAFE basically)
i'd happy if you answer me those question too :D

Regards, Mokhi.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: thread-unsafety problems as spl*() ones are NOP

2016-01-30 Thread Warner Losh 

> On Jan 30, 2016, at 9:37 PM, mokhi  wrote:
> 
> @imp So you think I should start to put locks there and see what happens? :)

I’d advocate a deeper understanding of the code. splXXX() is for code path
exclusion. Locks are for data protection. These are subtly different concepts
and may cause issues when you try to just blindly replace the splXXX with
a lock. You can start there, but you’ll still need to do code-path analysis to
see what the locks wind up protecting, and then see if there’s any access
to it outside of locked paths.

Warner



signature.asc
Description: Message signed with OpenPGP using GPGMail