Re: [Suggestion] drivers/tty: drivers/char/: for MAX_ASYNC_BUFFER_SIZE

From: Chen Gang <gang.chen@asianux.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-serial@vger.kernel.org, Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: Re: [Suggestion] drivers/tty: drivers/char/:  for MAX_ASYNC_BUFFER_SIZE
Date: Fri, 30 Nov 2012 10:52:38 +0800	[thread overview]
Message-ID: <50B81F76.8020508@asianux.com> (raw)
In-Reply-To: <20121129183207.GA4688@kroah.com>

于 2012年11月30日 02:32, Greg KH 写道:
> On Thu, Nov 29, 2012 at 01:57:59PM +0800, Chen Gang wrote:
>>> And, I really don't understand here, why do you want to change this?
>>> What is it going to change?  And why?
>>>
>>
>> Why:
>>   for the context MGSLPC_INFO *info in drivers/char/pcmcia/synclink_cs.c
>>     info->max_frame_size can be the value between 4096 .. 65535 (can be
>> set by its module input parameter)
>>     info->flag_buf length is 4096 (MAX_ASYNC_BUFFER_SIZE)
>>   in function rx_get_frame
>>     the framesize is limit by info->max_frame_size, but may still be
>> larger that 4096.
>>     when call function ldisc_receive_buf, info->flag_buf is equal to
>> 4096, but framesize can be more than 4096. it will cause memory over flow.
> 
> Do you use that pcmcia driver for anything?  Are those cards still
> around?

I am not use them.

I am just through code review (so it is only a suggestion).

this issue has effect with 4 synclink drivers
I checked their source code, all of them have the same issue.
  drivers/char/pcmcia/synclink_cs.c:213:        char flag_buf[MAX_ASYNC_BUFFER_SIZE];
  drivers/tty/synclink_gt.c:320:        char flag_buf[MAX_ASYNC_BUFFER_SIZE];
  drivers/tty/synclink.c:294:   char flag_buf[MAX_ASYNC_BUFFER_SIZE];
  drivers/tty/synclinkmp.c:265: char flag_buf[MAX_ASYNC_BUFFER_SIZE];

by the way, for the char_buf, has already useless (can be removed)
  drivers/tty/synclink_gt.c:321:        char char_buf[MAX_ASYNC_BUFFER_SIZE];
  drivers/tty/synclink.c:295:   char char_buf[MAX_ASYNC_BUFFER_SIZE];   
  drivers/tty/synclinkmp.c:266: char char_buf[MAX_ASYNC_BUFFER_SIZE];

> 
>> What:
>>   #define MAX_ASYNC_BUFFER_SIZE  0x10000 (instead of 4096, originally).
>>   let it match the max frame size.
>>
>> At last:
>>   my suggestion may be incorrect, need relative member (who expert about
>> it) to help checking.
> 
> That driver might be incorrect, yes, care to make up a patch for it and
> test it to verify it fixes the problem?
> 

and now Alan Cox has his own opinions
  at least, I think it is valuable to continue discussing about it.

if Alan Cox agree with it (but it seems not),  I will make patch, and try to perform test.
also welcome another members to help testing.

> thanks,
> 
> greg k-h
> 
> 

-- 
Chen Gang

Asianux Corporation