On Wed 2015-05-20 12:59:48, Marcin Niesluchowski wrote: > Reading message with dict may cause user buffer overflow due to > no limits of written dict and hardcoded user read buffer size. > As limits of dict are not clear, it may be possible in extreme > use case to trigger it (e.g. by driver passing some dict parameters > from userland or other module logging large key-value data). > > Truncate dict read by user when its size would cause user read > buffer to overflow. > > Bug was found during work on extension of kmsg enabling writing > dict from userspace.
By coincidence, Tejun has already provided slightly different patch for this problem, see https://lkml.org/lkml/2015/5/14/494 AFAIK, Tejun's patch already is in the -mm tree and thus on the way to get merged. I would prefer Tejun's solution. Best Regards, Petr > > Signed-off-by: Marcin Niesluchowski <m.niesluc...@samsung.com> > --- > kernel/printk/printk.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index c099b08..b61602d 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -505,6 +505,7 @@ int check_syslog_permissions(int type, bool from_file) > return security_syslog(type); > } > > +#define USER_READ_LOG_BUF_LEN 8192 > > /* /dev/kmsg - userspace message inject/listen interface */ > struct devkmsg_user { > @@ -512,7 +513,7 @@ struct devkmsg_user { > u32 idx; > enum log_flags prev; > struct mutex lock; > - char buf[8192]; > + char buf[USER_READ_LOG_BUF_LEN]; > }; > > static ssize_t devkmsg_write(struct kiocb *iocb, struct iov_iter *from) > @@ -648,21 +649,29 @@ static ssize_t devkmsg_read(struct file *file, char > __user *buf, > unsigned char c = log_dict(msg)[i]; > > if (line) { > + if (len >= USER_READ_LOG_BUF_LEN-1) > + break; > user->buf[len++] = ' '; > line = false; > } > > if (c == '\0') { > + if (len >= USER_READ_LOG_BUF_LEN-1) > + break; > user->buf[len++] = '\n'; > line = true; > continue; > } > > if (c < ' ' || c >= 127 || c == '\\') { > + if (len >= USER_READ_LOG_BUF_LEN-4) > + break; > len += sprintf(user->buf + len, "\\x%02x", c); > continue; > } > > + if (len >= USER_READ_LOG_BUF_LEN-1) > + break; > user->buf[len++] = c; > } > user->buf[len++] = '\n'; > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
