From: Chen Gang <gang.chen@asianux.com>
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [Suggestion] drivers/tty: drivers/char/: for MAX_ASYNC_BUFFER_SIZE
Date: Thu, 29 Nov 2012 13:07:28 +0800 [thread overview]
Message-ID: <50B6ED90.6060100@asianux.com> (raw)
In-Reply-To: <50B6E751.9000000@asianux.com>
Hello Greg Kroah-Hartman:
for MAX_ASYNC_BUFFER_SIZE:
it is defined as 4096;
but for the max buffer size which it processes, is 65535.
so suggest to #define MAX_ASYNC_BUFFER_SIZE 0x10000 (better than 0xffff)
I use 3 Step to prove it, please see below:
by the way:
I find it only through code review, not test it.
so I send it as suggestions (not a patch).
next time:
for the same case, if it is better to send a patch, directly.
please tell me by replying for this mail.
(at least, it can save your time, since you are busy)
gchen.
---------------------------------------------------------------------------------
Step 1:
the MAX_ASYNC_BUFFER_SIZE is 4096:
root@gchen-desktop:~/linux# grep -rn MAX_ASYNC_BUFFER_SIZE *
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_gt.c:321: char char_buf[MAX_ASYNC_BUFFER_SIZE];
drivers/tty/synclink.c:294: char flag_buf[MAX_ASYNC_BUFFER_SIZE];
drivers/tty/synclink.c:295: char char_buf[MAX_ASYNC_BUFFER_SIZE];
drivers/tty/synclinkmp.c:265: char flag_buf[MAX_ASYNC_BUFFER_SIZE];
drivers/tty/synclinkmp.c:266: char char_buf[MAX_ASYNC_BUFFER_SIZE];
include/uapi/linux/synclink.h:54:#define MAX_ASYNC_BUFFER_SIZE 4096
---------------------------------------------------------------------------------
Step 2:
one sample in drivers/char/pcmcia/synclink_cs.c: (same for another files)
we check the framesize according to info->max_frame_size in line 3687..3689.
and call ldisc_receive_buf without checking whether it is larger than MAX_ASYNC_BUFFER_SIZE at line 3705
info->max_frame_size can be the value between 4096 and 65535 in line 2729..2732.
ldisc_receive_buf call ld->ops->receive_buf to perform the work.
496 static void ldisc_receive_buf(struct tty_struct *tty,
497 const __u8 *data, char *flags, int count)
498 {
499 struct tty_ldisc *ld;
500 if (!tty)
501 return;
502 ld = tty_ldisc_ref(tty);
503 if (ld) {
504 if (ld->ops->receive_buf)
505 ld->ops->receive_buf(tty, data, flags, count);
506 tty_ldisc_deref(ld);
507 }
508 }
...
2728
2729 if (info->max_frame_size < 4096)
2730 info->max_frame_size = 4096;
2731 else if (info->max_frame_size > 65535)
2732 info->max_frame_size = 65535;
2733
...
3686 if (framesize) {
3687 if ((info->params.crc_type & HDLC_CRC_RETURN_EX &&
3688 framesize+1 > info->max_frame_size) ||
3689 framesize > info->max_frame_size)
3690 info->icount.rxlong++;
3691 else {
3692 if (status & BIT5)
3693 info->icount.rxok++;
3694
3695 if (info->params.crc_type & HDLC_CRC_RETURN_EX) {
3696 *(buf->data + framesize) = status & BIT5 ? RX_OK:RX_CRC_ERROR;
3697 ++framesize;
3698 }
3699
3700 #if SYNCLINK_GENERIC_HDLC
3701 if (info->netcount)
3702 hdlcdev_rx(info, buf->data, framesize);
3703 else
3704 #endif
3705 ldisc_receive_buf(tty, buf->data, info->flag_buf, framesize);
3706 }
3707 }
---------------------------------------------------------------------------------
Step 3:
one sample in drivers/tty/n_gsm.c (same for another implementation)
receive_buf is a function ptr which may be gsmld_receive_buf at line 2819.
it does not check the length of count whether larger than MAX_ASYNC_BUFFER_SIZE.
if count is larger than MAX_ASYNC_BUFFER_SIZE, will cause issue.
it is only a sample, maybe not the function ptr which mentioned in Step 2.
at lease, I have checked 3 function ptr implementations, none of them check the MAX_ASYNC_BUFFER_SIZE.
the all function ptr searching is at the bottom.
2257 static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
2258 char *fp, int count)
2259 {
2260 struct gsm_mux *gsm = tty->disc_data;
2261 const unsigned char *dp;
2262 char *f;
2263 int i;
2264 char buf[64];
2265 char flags;
2266
2267 if (debug & 4)
2268 print_hex_dump_bytes("gsmld_receive: ", DUMP_PREFIX_OFFSET,
2269 cp, count);
2270
2271 for (i = count, dp = cp, f = fp; i; i--, dp++) {
2272 flags = *f++;
2273 switch (flags) {
2274 case TTY_NORMAL:
2275 gsm->receive(gsm, *dp);
2276 break;
2277 case TTY_OVERRUN:
2278 case TTY_BREAK:
2279 case TTY_PARITY:
2280 case TTY_FRAME:
2281 gsm->error(gsm, *dp, flags);
2282 break;
2283 default:
2284 WARN_ONCE(1, "%s: unknown flag %d\n",
2285 tty_name(tty, buf), flags);
2286 break;
2287 }
2288 }
2289 /* FASYNC if needed ? */
2290 /* If clogged call tty_throttle(tty); */
2291 }
2292
...
2806 /* Line discipline for real tty */
2807 struct tty_ldisc_ops tty_ldisc_packet = {
2808 .owner = THIS_MODULE,
2809 .magic = TTY_LDISC_MAGIC,
2810 .name = "n_gsm",
2811 .open = gsmld_open,
2812 .close = gsmld_close,
2813 .flush_buffer = gsmld_flush_buffer,
2814 .chars_in_buffer = gsmld_chars_in_buffer,
2815 .read = gsmld_read,
2816 .write = gsmld_write,
2817 .ioctl = gsmld_ioctl,
2818 .poll = gsmld_poll,
2819 .receive_buf = gsmld_receive_buf,
2820 .write_wakeup = gsmld_write_wakeup
2821 };
2822
input/serio/serport.c:244: .receive_buf = serport_ldisc_receive,
isdn/gigaset/ser-gigaset.c:758: .receive_buf = gigaset_tty_receive,
misc/ti-st/st_core.c:822: .receive_buf = st_tty_receive,
net/can/slcan.c:626: .receive_buf = slcan_receive_buf,
net/hamradio/6pack.c:808: .receive_buf = sixpack_receive_buf,
net/hamradio/mkiss.c:996: .receive_buf = mkiss_receive_buf,
net/irda/irtty-sir.c:547: .receive_buf = irtty_receive_buf,
net/caif/caif_serial.c:378: .receive_buf = ldisc_receive,
net/ppp/ppp_synctty.c:428: .receive_buf = ppp_sync_receive,
net/ppp/ppp_async.c:387: .receive_buf = ppp_asynctty_receive,
tty/n_gsm.c:2819: .receive_buf = gsmld_receive_buf,
tty/n_tty.c:2129: .receive_buf = n_tty_receive_buf,
tty/n_hdlc.c:234: .receive_buf = n_hdlc_tty_receive,
tty/n_tracerouter.c:193: .receive_buf = n_tracerouter_receivebuf
tty/n_r3964.c:156: .receive_buf = r3964_receive_buf,
next parent reply other threads:[~2012-11-29 5:06 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <50B6E751.9000000@asianux.com>
2012-11-29 5:07 ` Chen Gang [this message]
2012-11-29 13:41 ` [Suggestion] drivers/tty: drivers/char/: for MAX_ASYNC_BUFFER_SIZE Alan Cox
2012-11-30 2:27 ` Chen Gang
2012-11-30 3:39 ` Chen Gang
2012-11-29 5:13 ` 【Suggestion】drivers/tty: " Greg KH
2012-11-29 5:57 ` [Suggestion] drivers/tty: " Chen Gang
2012-11-29 6:14 ` [PATCH] MAINTAINERS: TTY - Add linux-serial mailing list Joe Perches
2012-11-29 6:27 ` Chen Gang
2012-11-29 8:23 ` Jiri Slaby
2012-11-29 18:32 ` [Suggestion] drivers/tty: drivers/char/: for MAX_ASYNC_BUFFER_SIZE Greg KH
2012-11-30 2:52 ` Chen Gang
[not found] ` <C7D3911F-7B6B-4353-A84B-0218FAB27198@microgate.com>
2012-11-30 6:28 ` Chen Gang
2012-11-30 7:14 ` Chen Gang
2012-11-30 16:24 ` Paul Fulghum
2012-11-30 19:46 ` [PATCH] synclink fix ldisc buffer argument Paul Fulghum
2012-12-02 15:13 ` Alan Cox
[not found] ` <F6B8A325-7DBF-4623-B16C-CDC5642EFD16@microgate.com>
2012-12-02 18:10 ` Alan Cox
[not found] ` <989CB961-79F8-479B-B16C-41358A60AC94@microgate.com>
2012-12-03 2:20 ` Chen Gang
2012-12-03 16:03 ` Paul Fulghum
2012-12-05 1:57 ` Chen Gang
2012-12-19 2:23 ` Chen Gang
2012-12-19 4:09 ` Greg KH
2012-12-19 4:10 ` Chen Gang
2012-12-20 4:16 ` [PATCH] drivers/tty/synclink: let receive buffer size match max frame size Chen Gang
2012-12-03 17:13 ` [PATCH] synclink fix ldisc buffer argument Paul Fulghum
2012-12-05 1:35 ` Chen Gang
2012-12-07 2:15 ` Chen Gang
2012-12-10 1:32 ` [Consult]: " Chen Gang
2012-12-01 9:01 ` [Suggestion] drivers/tty: drivers/char/: for MAX_ASYNC_BUFFER_SIZE Chen Gang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50B6ED90.6060100@asianux.com \
--to=gang.chen@asianux.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox