| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 9 Nov 2007 16:44:47 -0600 From: "Steve French" <smfrench@...il.com> To: "Przemyslaw Wegrzyn" <czajnik@...jsoft.pl> Cc: "Andrew Morton" <akpm@...ux-foundation.org>, LKML <linux-kernel@...r.kernel.org>, joern@...fs.org, linux-cifs-client@...ts.samba.org Subject: Re: 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@...jsoft.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@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists