lkml.org 
[lkml]   [2007]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Fw: Buffer overflow in CIFS VFS.
I have done an analysis of the SMB functions (56 callers of
SendReceive, 4 of SendReceive2 and 2 callers of
SendReceiveBlockingLock) and found additional changes which should
help performance, by reducing the number of expensive large buffer
allocations and also by freeing buffers back to the pool faster. See
below. The obvious need is to create an SendReceive-NoResponse (or
equivalent) which
frees the SMB request buffer after send, and does not copy into an smb
response buffer. The following functions need to be changed to use
that (and also address the problem that Przemyslaw noted):

TreeDisconnect, uLogoff, Close, findClose, SetFileSize, SetFileTimes,
(Lock and PosixLock need slightly more complicated change
since only some of their paths can use proposed new function)

There is a longer list of functions which should be optimized to call
a SendReceiveNoResponse like function as well (but we can do this
later after 2.6.24) since we also do not parse their response SMB (it
would make buffer allocation more efficient to free the request
buffers faster and avoid a memcpy):

PosixDelFile, DelFile, Rmdir, MkDir, Rename, RenameOpenFile, Copy,
CreateSymLink, CreateHardLinkUnix, CreateHardLink, SetPosixACL,
SetFSUnixInfo (also change to small buf allocate), SetEOF, SetTimes,
SetAttrLegacy, UnixSetPerms, SetEA

Some other functions can be changed to small buffer alloc requests:
Negotiate, SetFSUnixInfo, QueryReparseLink, GetExtAttr, the 6 QFSInfos
requests, Notify


Explanatory Key for the list below:
-------------------------------------------------
SR = the function currently uses SendReceive
SR2 = uses SendReceive2()
SRB = uses SendReceiveBlockingLock
large = uses smb_init (allocates large buffer)
small = uses small_smb_init (allocates small buffer)

LR = getting a large response back from the server is common
NR = no return SMB needed to be parsed by caller, processing only
depends on value of SMB error
ID = idempotent (does not change state significantly if repeated)
L = can be long operation (and take longer than a few seconds)
B = operation may block indefinitely
PB = operation has variable length request size due to path name in request
IOV = needs to send iovec

List of SMB Functions and their characteristics
-------------------------------------------------------------
(in fs/cifs/cifssmb.c)
Negotiate large LR*, ID
TreeDisconnect small SR NR
uLogoff small SR NR
PosixDelFile large SR PB,NR
DelFile large SR PB,NR
RmDir large SR PB,NR
MkDir large SR PB,NR
PosixCreate large SR PB
LegacyOpen large SR PB
Open large SR PB
Read small SR2 LR, ID
Write large SR (need mixed user/kernel IOV to change) L, ID?
Write2 small SR2 IOV, L
Lock small SR/SRB NR, B
PosixLock small SR/SRB (NR on setpath), B
Close small SR NR
Rename large SR NR, PB
RenameOpenFile large SR NR, PB
Copy large SR NR, PB
CreateSymLink large SR NR, PB
CreateHardLinkU large SR NR, PB
CreateHardLink large SR NR, PB
QuerySymLink large SR LR, PB, ID
QueryReparseLk large SR LR, ID
QueryPosixACL large SR LR, PB, ID
SetPosixACL large SR NR, PB
GetExtAttr large SR LR, ID
GetCIFSACL small SR2 LR, ID
QueryInformatn large SR PB, ID
QPathInfo large SR PB, ID
UnixQPathInfo large SR PB, ID
FindSingle large SR PB, LR?, ID (remove)
FindFirst large SR PB, LR
FindNext large SR PB*, LR, ID* (resume key)
FindClose small SR NR
GetSrvInodeNum large SR PB, ID
GetDFSRefer large SR PB, LR, ID
OldQFSInfo large SR ID
QFSInfo large SR ID
QFSAttribute large SR ID
QFSDeviceInfo large SR ID
QFSUnixInfo large SR ID
SetFSUnixInfo large SR ID, NR
QFSPosixInfo large SR ID
SetEOF large SR PB, NR
SetFileSize small SR NR
SetFileTimes small SR NR
SetTimes large SR PB, NR
SetAttrLegacy large SR PB, NR
UnixSetPerms large SR PB, NR
Notify large SR
QueryAllEAs large SR PB, LR, ID
QueryEA large SR PB, LR, ID
SetEA large SR PB, NR

(in fs/cifs/sess.c)
SessionSetup large SR2 IOV, large request, LR

(in fs/cifs/connect.c)
TreeConnect large SR PB
other uses of SendReceive* in this file are obsolete

(in fs/cifs/transport.c)
send_lock_cancel no buffer allocate, SR NR function looks broken



On Nov 9, 2007 4:59 AM, Przemyslaw Wegrzyn <czajnik@czajsoft.pl> wrote:
> to use SendReceive2 in places where kvec is really needed. Also these
> functions are almost identical, maybe SendReceive should wrap
> SendReceive2 preparing kvec with a buffer pointer passed to it?
>
> Obviously it is up to you, as a maintainer. I'd prefer adding a small
> header to each buffer with the buffer size and perhaps a type, or even a
> destructor function pointer. Simple macros could be used to obtain
> buffer size, given the buffer body pointer, or to dispose the buffer.
> That would save from checking the buffer type all over the code
> explicitly, or even worse, make strange assumptions about the type of
> buffer being passed - as we can see this is error-prone. That for a
> little cost of a few additional bytes per buffer.
That might be better, although without memory pools, this would perform
much worse

>



--
Thanks,

Steve
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-11-09 23:47    [W:0.349 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site