| Message ID | 20130117074405.GA26270@elgon.mountain |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi Dan, Am 17.01.2013 08:44, schrieb Dan Carpenter: > If l == 12 then later we subtract 12 leaving zero. We do a zero size > allocation, so "dbgline" points to the ZERO_SIZE_PTR. It leads to an > oops when we set the NUL terminator: > dbgline[3 * l - 1] = '\0'; thanks for finding that bug, but NAK to your fix. > @@ -239,7 +239,7 @@ static inline void dump_rawmsg(enum debuglevel level, const char *tag, > return; > > l = CAPIMSG_LEN(data); > - if (l < 12) { > + if (l <= 12) { > gig_dbg(level, "%s: ??? LEN=%04d", tag, l); > return; > } CAPI messages of exactly 12 bytes are legal, and should be decoded with the regular gig_dbg() call immediately after that hunk. It's just the hex dump part that should be skipped in that case. So I'd prefer to have it fixed this way instead: @@ -248,6 +248,8 @@ static inline void dump_rawmsg(enum debuglevel level, const char *tag, CAPIMSG_APPID(data), CAPIMSG_MSGID(data), l, CAPIMSG_CONTROL(data)); l -= 12; + if (l <= 0) + return; dbgline = kmalloc(3 * l, GFP_ATOMIC); if (!dbgline) return; I'll prepare a patch of my own, citing you as reporter, if that's ok with you. Thanks, Tilman
On Fri, Jan 18, 2013 at 01:18:00PM +0100, Tilman Schmidt wrote: > I'll prepare a patch of my own, citing you as reporter, if that's > ok with you. > That sounds great. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c index 68452b7..0d34325 100644 --- a/drivers/isdn/gigaset/capi.c +++ b/drivers/isdn/gigaset/capi.c @@ -239,7 +239,7 @@ static inline void dump_rawmsg(enum debuglevel level, const char *tag, return; l = CAPIMSG_LEN(data); - if (l < 12) { + if (l <= 12) { gig_dbg(level, "%s: ??? LEN=%04d", tag, l); return; }
If l == 12 then later we subtract 12 leaving zero. We do a zero size allocation, so "dbgline" points to the ZERO_SIZE_PTR. It leads to an oops when we set the NUL terminator: dbgline[3 * l - 1] = '\0'; Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- Static analysis stuff. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html