| Message ID | 20190219122746.4197-4-kai.heng.feng@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | CVE-2019-3460 - Heap data infoleak in multiple locations including functionl2cap_parse_conf_rsp | expand |
On 2019-02-19 20:27:46, Kai-Heng Feng wrote: > From: Marcel Holtmann <marcel@holtmann.org> > > When doing option parsing for standard type values of 1, 2 or 4 octets, > the value is converted directly into a variable instead of a pointer. To > avoid being tricked into being a pointer, check that for these option > types that sizes actually match. In L2CAP every option is fixed size and > thus it is prudent anyway to ensure that the remote side sends us the > right option size along with option paramters. > > If the option size is not matching the option type, then that option is > silently ignored. It is a protocol violation and instead of trying to > give the remote attacker any further hints just pretend that option is > not present and proceed with the default values. Implementation > following the specification and its qualification procedures will always > use the correct size and thus not being impacted here. > > To keep the code readable and consistent accross all options, a few > cosmetic changes were also required. > > CVE-2019-3460 > > Signed-off-by: Marcel Holtmann <marcel@holtmann.org> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> > (backported from commit af3d5d1c87664a4f150fcf3534c6567cb19909b0 linux-next) Could you give a little info about what kind of backporting effort this required? I quickly compared this patch to the upstream patch and it mostly looked like contextual changes but I'd like for you to confirm that. Best practice would be to leave a little comment above your Signed-off-by line. You're probably already aware of this but here's the in-tree documentation on this practice: If you are a subsystem or branch maintainer, sometimes you need to slightly modify patches you receive in order to merge them, because the code is not exactly the same in your tree and the submitters’. If you stick strictly to rule (c), you should ask the submitter to rediff, but this is a totally counter-productive waste of time and energy. Rule (b) allows you to adjust the code, but then it is very impolite to change one submitter’s code and make him endorse your bugs. To solve this problem, it is recommended that you add a line between the last Signed-off-by header and yours, indicating the nature of your changes. While there is nothing mandatory about this, it seems like prepending the description with your mail and/or name, all enclosed in square brackets, is noticeable enough to make it obvious that you are responsible for last-minute changes. Example: Signed-off-by: Random J Developer <random@developer.example.org> [lucky@maintainer.example.org: struct foo moved from foo.c to foo.h] Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org> This practice is particularly helpful if you maintain a stable branch and want at the same time to credit the author, track changes, merge the fix, and protect the submitter from complaints. Note that under no circumstances can you change the author’s identity (the From header), as it is the one which appears in the changelog. - https://www.kernel.org/doc/html/latest/process/submitting-patches.html Tyler > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > net/bluetooth/l2cap_core.c | 77 +++++++++++++++++++++++--------------- > 1 file changed, 46 insertions(+), 31 deletions(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 61d0f290c0c6..a7de6cbdeced 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -3284,10 +3284,14 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > > switch (type) { > case L2CAP_CONF_MTU: > + if (olen != 2) > + break; > mtu = val; > break; > > case L2CAP_CONF_FLUSH_TO: > + if (olen != 2) > + break; > chan->flush_to = val; > break; > > @@ -3295,26 +3299,30 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > break; > > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *) val, olen); > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *) val, olen); > break; > > case L2CAP_CONF_FCS: > + if (olen != 1) > + break; > if (val == L2CAP_FCS_NONE) > set_bit(CONF_RECV_NO_FCS, &chan->conf_state); > break; > > case L2CAP_CONF_EFS: > - if (olen == sizeof(efs)) { > - remote_efs = 1; > - memcpy(&efs, (void *) val, olen); > - } > + if (olen != sizeof(efs)) > + break; > + remote_efs = 1; > + memcpy(&efs, (void *) val, olen); > break; > > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > if (!chan->conn->hs_enabled) > return -ECONNREFUSED; > - > set_bit(FLAG_EXT_CTRL, &chan->flags); > set_bit(CONF_EWS_RECV, &chan->conf_state); > chan->tx_win_max = L2CAP_DEFAULT_EXT_WINDOW; > @@ -3324,7 +3332,6 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > default: > if (hint) > break; > - > result = L2CAP_CONF_UNKNOWN; > *((u8 *) ptr++) = type; > break; > @@ -3492,55 +3499,60 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len, > > switch (type) { > case L2CAP_CONF_MTU: > + if (olen != 2) > + break; > if (val < L2CAP_DEFAULT_MIN_MTU) { > *result = L2CAP_CONF_UNACCEPT; > chan->imtu = L2CAP_DEFAULT_MIN_MTU; > } else > chan->imtu = val; > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, > + endptr - ptr); > break; > > case L2CAP_CONF_FLUSH_TO: > + if (olen != 2) > + break; > chan->flush_to = val; > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, > - 2, chan->flush_to, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, > + chan->flush_to, endptr - ptr); > break; > > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *)val, olen); > - > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *)val, olen); > if (test_bit(CONF_STATE2_DEVICE, &chan->conf_state) && > rfc.mode != chan->mode) > return -ECONNREFUSED; > - > chan->fcs = 0; > - > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, > - sizeof(rfc), (unsigned long) &rfc, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc), > + (unsigned long) &rfc, endptr - ptr); > break; > > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > chan->ack_win = min_t(u16, val, chan->ack_win); > l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 2, > chan->tx_win, endptr - ptr); > break; > > case L2CAP_CONF_EFS: > - if (olen == sizeof(efs)) { > - memcpy(&efs, (void *)val, olen); > - > - if (chan->local_stype != L2CAP_SERV_NOTRAFIC && > - efs.stype != L2CAP_SERV_NOTRAFIC && > - efs.stype != chan->local_stype) > - return -ECONNREFUSED; > - > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), > - (unsigned long) &efs, endptr - ptr); > - } > + if (olen != sizeof(efs)) > + break; > + memcpy(&efs, (void *)val, olen); > + if (chan->local_stype != L2CAP_SERV_NOTRAFIC && > + efs.stype != L2CAP_SERV_NOTRAFIC && > + efs.stype != chan->local_stype) > + return -ECONNREFUSED; > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), > + (unsigned long) &efs, endptr - ptr); > break; > > case L2CAP_CONF_FCS: > + if (olen != 1) > + break; > if (*result == L2CAP_CONF_PENDING) > if (val == L2CAP_FCS_NONE) > set_bit(CONF_RECV_NO_FCS, > @@ -3655,10 +3667,13 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len) > > switch (type) { > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *)val, olen); > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *)val, olen); > break; > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > txwin_ext = val; > break; > } > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
> On Feb 20, 2019, at 9:16 AM, Tyler Hicks <tyhicks@canonical.com> wrote: > > On 2019-02-19 20:27:46, Kai-Heng Feng wrote: >> From: Marcel Holtmann <marcel@holtmann.org> >> >> When doing option parsing for standard type values of 1, 2 or 4 octets, >> the value is converted directly into a variable instead of a pointer. To >> avoid being tricked into being a pointer, check that for these option >> types that sizes actually match. In L2CAP every option is fixed size and >> thus it is prudent anyway to ensure that the remote side sends us the >> right option size along with option paramters. >> >> If the option size is not matching the option type, then that option is >> silently ignored. It is a protocol violation and instead of trying to >> give the remote attacker any further hints just pretend that option is >> not present and proceed with the default values. Implementation >> following the specification and its qualification procedures will always >> use the correct size and thus not being impacted here. >> >> To keep the code readable and consistent accross all options, a few >> cosmetic changes were also required. >> >> CVE-2019-3460 >> >> Signed-off-by: Marcel Holtmann <marcel@holtmann.org> >> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> >> (backported from commit af3d5d1c87664a4f150fcf3534c6567cb19909b0 linux-next) > > Could you give a little info about what kind of backporting effort this > required? I quickly compared this patch to the upstream patch and it > mostly looked like contextual changes but I'd like for you to confirm > that. Yes, it’s just contextual change. > > Best practice would be to leave a little comment above your > Signed-off-by line. You're probably already aware of this but here's the > in-tree documentation on this practice: I’ve never added bracket comments in the past, but this is a good practice. I’ll do this in the future. Do you want me to re-send a V2? Kai-Heng > > If you are a subsystem or branch maintainer, sometimes you need to > slightly modify patches you receive in order to merge them, because the > code is not exactly the same in your tree and the submitters’. If you > stick strictly to rule (c), you should ask the submitter to rediff, but > this is a totally counter-productive waste of time and energy. Rule (b) > allows you to adjust the code, but then it is very impolite to change > one submitter’s code and make him endorse your bugs. To solve this > problem, it is recommended that you add a line between the last > Signed-off-by header and yours, indicating the nature of your changes. > While there is nothing mandatory about this, it seems like prepending > the description with your mail and/or name, all enclosed in square > brackets, is noticeable enough to make it obvious that you are > responsible for last-minute changes. Example: > > Signed-off-by: Random J Developer <random@developer.example.org> > [lucky@maintainer.example.org: struct foo moved from foo.c to foo.h] > Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org> > > This practice is particularly helpful if you maintain a stable branch > and want at the same time to credit the author, track changes, merge the > fix, and protect the submitter from complaints. Note that under no > circumstances can you change the author’s identity (the From header), as > it is the one which appears in the changelog. > > - https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > Tyler > >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> >> --- >> net/bluetooth/l2cap_core.c | 77 +++++++++++++++++++++++--------------- >> 1 file changed, 46 insertions(+), 31 deletions(-) >> >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c >> index 61d0f290c0c6..a7de6cbdeced 100644 >> --- a/net/bluetooth/l2cap_core.c >> +++ b/net/bluetooth/l2cap_core.c >> @@ -3284,10 +3284,14 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data >> >> switch (type) { >> case L2CAP_CONF_MTU: >> + if (olen != 2) >> + break; >> mtu = val; >> break; >> >> case L2CAP_CONF_FLUSH_TO: >> + if (olen != 2) >> + break; >> chan->flush_to = val; >> break; >> >> @@ -3295,26 +3299,30 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data >> break; >> >> case L2CAP_CONF_RFC: >> - if (olen == sizeof(rfc)) >> - memcpy(&rfc, (void *) val, olen); >> + if (olen != sizeof(rfc)) >> + break; >> + memcpy(&rfc, (void *) val, olen); >> break; >> >> case L2CAP_CONF_FCS: >> + if (olen != 1) >> + break; >> if (val == L2CAP_FCS_NONE) >> set_bit(CONF_RECV_NO_FCS, &chan->conf_state); >> break; >> >> case L2CAP_CONF_EFS: >> - if (olen == sizeof(efs)) { >> - remote_efs = 1; >> - memcpy(&efs, (void *) val, olen); >> - } >> + if (olen != sizeof(efs)) >> + break; >> + remote_efs = 1; >> + memcpy(&efs, (void *) val, olen); >> break; >> >> case L2CAP_CONF_EWS: >> + if (olen != 2) >> + break; >> if (!chan->conn->hs_enabled) >> return -ECONNREFUSED; >> - >> set_bit(FLAG_EXT_CTRL, &chan->flags); >> set_bit(CONF_EWS_RECV, &chan->conf_state); >> chan->tx_win_max = L2CAP_DEFAULT_EXT_WINDOW; >> @@ -3324,7 +3332,6 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data >> default: >> if (hint) >> break; >> - >> result = L2CAP_CONF_UNKNOWN; >> *((u8 *) ptr++) = type; >> break; >> @@ -3492,55 +3499,60 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len, >> >> switch (type) { >> case L2CAP_CONF_MTU: >> + if (olen != 2) >> + break; >> if (val < L2CAP_DEFAULT_MIN_MTU) { >> *result = L2CAP_CONF_UNACCEPT; >> chan->imtu = L2CAP_DEFAULT_MIN_MTU; >> } else >> chan->imtu = val; >> - l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, endptr - ptr); >> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, >> + endptr - ptr); >> break; >> >> case L2CAP_CONF_FLUSH_TO: >> + if (olen != 2) >> + break; >> chan->flush_to = val; >> - l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, >> - 2, chan->flush_to, endptr - ptr); >> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, >> + chan->flush_to, endptr - ptr); >> break; >> >> case L2CAP_CONF_RFC: >> - if (olen == sizeof(rfc)) >> - memcpy(&rfc, (void *)val, olen); >> - >> + if (olen != sizeof(rfc)) >> + break; >> + memcpy(&rfc, (void *)val, olen); >> if (test_bit(CONF_STATE2_DEVICE, &chan->conf_state) && >> rfc.mode != chan->mode) >> return -ECONNREFUSED; >> - >> chan->fcs = 0; >> - >> - l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, >> - sizeof(rfc), (unsigned long) &rfc, endptr - ptr); >> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc), >> + (unsigned long) &rfc, endptr - ptr); >> break; >> >> case L2CAP_CONF_EWS: >> + if (olen != 2) >> + break; >> chan->ack_win = min_t(u16, val, chan->ack_win); >> l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 2, >> chan->tx_win, endptr - ptr); >> break; >> >> case L2CAP_CONF_EFS: >> - if (olen == sizeof(efs)) { >> - memcpy(&efs, (void *)val, olen); >> - >> - if (chan->local_stype != L2CAP_SERV_NOTRAFIC && >> - efs.stype != L2CAP_SERV_NOTRAFIC && >> - efs.stype != chan->local_stype) >> - return -ECONNREFUSED; >> - >> - l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), >> - (unsigned long) &efs, endptr - ptr); >> - } >> + if (olen != sizeof(efs)) >> + break; >> + memcpy(&efs, (void *)val, olen); >> + if (chan->local_stype != L2CAP_SERV_NOTRAFIC && >> + efs.stype != L2CAP_SERV_NOTRAFIC && >> + efs.stype != chan->local_stype) >> + return -ECONNREFUSED; >> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), >> + (unsigned long) &efs, endptr - ptr); >> break; >> >> case L2CAP_CONF_FCS: >> + if (olen != 1) >> + break; >> if (*result == L2CAP_CONF_PENDING) >> if (val == L2CAP_FCS_NONE) >> set_bit(CONF_RECV_NO_FCS, >> @@ -3655,10 +3667,13 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len) >> >> switch (type) { >> case L2CAP_CONF_RFC: >> - if (olen == sizeof(rfc)) >> - memcpy(&rfc, (void *)val, olen); >> + if (olen != sizeof(rfc)) >> + break; >> + memcpy(&rfc, (void *)val, olen); >> break; >> case L2CAP_CONF_EWS: >> + if (olen != 2) >> + break; >> txwin_ext = val; >> break; >> } >> -- >> 2.17.1 >> >> >> -- >> kernel-team mailing list >> kernel-team@lists.ubuntu.com >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 2019-02-20 09:36:29, Kai Heng Feng wrote: > > > > On Feb 20, 2019, at 9:16 AM, Tyler Hicks <tyhicks@canonical.com> wrote: > > > > On 2019-02-19 20:27:46, Kai-Heng Feng wrote: > >> From: Marcel Holtmann <marcel@holtmann.org> > >> > >> When doing option parsing for standard type values of 1, 2 or 4 octets, > >> the value is converted directly into a variable instead of a pointer. To > >> avoid being tricked into being a pointer, check that for these option > >> types that sizes actually match. In L2CAP every option is fixed size and > >> thus it is prudent anyway to ensure that the remote side sends us the > >> right option size along with option paramters. > >> > >> If the option size is not matching the option type, then that option is > >> silently ignored. It is a protocol violation and instead of trying to > >> give the remote attacker any further hints just pretend that option is > >> not present and proceed with the default values. Implementation > >> following the specification and its qualification procedures will always > >> use the correct size and thus not being impacted here. > >> > >> To keep the code readable and consistent accross all options, a few > >> cosmetic changes were also required. > >> > >> CVE-2019-3460 > >> > >> Signed-off-by: Marcel Holtmann <marcel@holtmann.org> > >> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> > >> (backported from commit af3d5d1c87664a4f150fcf3534c6567cb19909b0 linux-next) > > > > Could you give a little info about what kind of backporting effort this > > required? I quickly compared this patch to the upstream patch and it > > mostly looked like contextual changes but I'd like for you to confirm > > that. > > Yes, it’s just contextual change. > > > > > Best practice would be to leave a little comment above your > > Signed-off-by line. You're probably already aware of this but here's the > > in-tree documentation on this practice: > > I’ve never added bracket comments in the past, but this is a good practice. > I’ll do this in the future. Thanks! > > Do you want me to re-send a V2? No, I don't think there's a need for a v2. Tyler > > Kai-Heng > > > > > If you are a subsystem or branch maintainer, sometimes you need to > > slightly modify patches you receive in order to merge them, because the > > code is not exactly the same in your tree and the submitters’. If you > > stick strictly to rule (c), you should ask the submitter to rediff, but > > this is a totally counter-productive waste of time and energy. Rule (b) > > allows you to adjust the code, but then it is very impolite to change > > one submitter’s code and make him endorse your bugs. To solve this > > problem, it is recommended that you add a line between the last > > Signed-off-by header and yours, indicating the nature of your changes. > > While there is nothing mandatory about this, it seems like prepending > > the description with your mail and/or name, all enclosed in square > > brackets, is noticeable enough to make it obvious that you are > > responsible for last-minute changes. Example: > > > > Signed-off-by: Random J Developer <random@developer.example.org> > > [lucky@maintainer.example.org: struct foo moved from foo.c to foo.h] > > Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org> > > > > This practice is particularly helpful if you maintain a stable branch > > and want at the same time to credit the author, track changes, merge the > > fix, and protect the submitter from complaints. Note that under no > > circumstances can you change the author’s identity (the From header), as > > it is the one which appears in the changelog. > > > > - https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > > > Tyler > > > >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > >> --- > >> net/bluetooth/l2cap_core.c | 77 +++++++++++++++++++++++--------------- > >> 1 file changed, 46 insertions(+), 31 deletions(-) > >> > >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > >> index 61d0f290c0c6..a7de6cbdeced 100644 > >> --- a/net/bluetooth/l2cap_core.c > >> +++ b/net/bluetooth/l2cap_core.c > >> @@ -3284,10 +3284,14 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > >> > >> switch (type) { > >> case L2CAP_CONF_MTU: > >> + if (olen != 2) > >> + break; > >> mtu = val; > >> break; > >> > >> case L2CAP_CONF_FLUSH_TO: > >> + if (olen != 2) > >> + break; > >> chan->flush_to = val; > >> break; > >> > >> @@ -3295,26 +3299,30 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > >> break; > >> > >> case L2CAP_CONF_RFC: > >> - if (olen == sizeof(rfc)) > >> - memcpy(&rfc, (void *) val, olen); > >> + if (olen != sizeof(rfc)) > >> + break; > >> + memcpy(&rfc, (void *) val, olen); > >> break; > >> > >> case L2CAP_CONF_FCS: > >> + if (olen != 1) > >> + break; > >> if (val == L2CAP_FCS_NONE) > >> set_bit(CONF_RECV_NO_FCS, &chan->conf_state); > >> break; > >> > >> case L2CAP_CONF_EFS: > >> - if (olen == sizeof(efs)) { > >> - remote_efs = 1; > >> - memcpy(&efs, (void *) val, olen); > >> - } > >> + if (olen != sizeof(efs)) > >> + break; > >> + remote_efs = 1; > >> + memcpy(&efs, (void *) val, olen); > >> break; > >> > >> case L2CAP_CONF_EWS: > >> + if (olen != 2) > >> + break; > >> if (!chan->conn->hs_enabled) > >> return -ECONNREFUSED; > >> - > >> set_bit(FLAG_EXT_CTRL, &chan->flags); > >> set_bit(CONF_EWS_RECV, &chan->conf_state); > >> chan->tx_win_max = L2CAP_DEFAULT_EXT_WINDOW; > >> @@ -3324,7 +3332,6 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > >> default: > >> if (hint) > >> break; > >> - > >> result = L2CAP_CONF_UNKNOWN; > >> *((u8 *) ptr++) = type; > >> break; > >> @@ -3492,55 +3499,60 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len, > >> > >> switch (type) { > >> case L2CAP_CONF_MTU: > >> + if (olen != 2) > >> + break; > >> if (val < L2CAP_DEFAULT_MIN_MTU) { > >> *result = L2CAP_CONF_UNACCEPT; > >> chan->imtu = L2CAP_DEFAULT_MIN_MTU; > >> } else > >> chan->imtu = val; > >> - l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, endptr - ptr); > >> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, > >> + endptr - ptr); > >> break; > >> > >> case L2CAP_CONF_FLUSH_TO: > >> + if (olen != 2) > >> + break; > >> chan->flush_to = val; > >> - l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, > >> - 2, chan->flush_to, endptr - ptr); > >> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, > >> + chan->flush_to, endptr - ptr); > >> break; > >> > >> case L2CAP_CONF_RFC: > >> - if (olen == sizeof(rfc)) > >> - memcpy(&rfc, (void *)val, olen); > >> - > >> + if (olen != sizeof(rfc)) > >> + break; > >> + memcpy(&rfc, (void *)val, olen); > >> if (test_bit(CONF_STATE2_DEVICE, &chan->conf_state) && > >> rfc.mode != chan->mode) > >> return -ECONNREFUSED; > >> - > >> chan->fcs = 0; > >> - > >> - l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, > >> - sizeof(rfc), (unsigned long) &rfc, endptr - ptr); > >> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc), > >> + (unsigned long) &rfc, endptr - ptr); > >> break; > >> > >> case L2CAP_CONF_EWS: > >> + if (olen != 2) > >> + break; > >> chan->ack_win = min_t(u16, val, chan->ack_win); > >> l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 2, > >> chan->tx_win, endptr - ptr); > >> break; > >> > >> case L2CAP_CONF_EFS: > >> - if (olen == sizeof(efs)) { > >> - memcpy(&efs, (void *)val, olen); > >> - > >> - if (chan->local_stype != L2CAP_SERV_NOTRAFIC && > >> - efs.stype != L2CAP_SERV_NOTRAFIC && > >> - efs.stype != chan->local_stype) > >> - return -ECONNREFUSED; > >> - > >> - l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), > >> - (unsigned long) &efs, endptr - ptr); > >> - } > >> + if (olen != sizeof(efs)) > >> + break; > >> + memcpy(&efs, (void *)val, olen); > >> + if (chan->local_stype != L2CAP_SERV_NOTRAFIC && > >> + efs.stype != L2CAP_SERV_NOTRAFIC && > >> + efs.stype != chan->local_stype) > >> + return -ECONNREFUSED; > >> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), > >> + (unsigned long) &efs, endptr - ptr); > >> break; > >> > >> case L2CAP_CONF_FCS: > >> + if (olen != 1) > >> + break; > >> if (*result == L2CAP_CONF_PENDING) > >> if (val == L2CAP_FCS_NONE) > >> set_bit(CONF_RECV_NO_FCS, > >> @@ -3655,10 +3667,13 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len) > >> > >> switch (type) { > >> case L2CAP_CONF_RFC: > >> - if (olen == sizeof(rfc)) > >> - memcpy(&rfc, (void *)val, olen); > >> + if (olen != sizeof(rfc)) > >> + break; > >> + memcpy(&rfc, (void *)val, olen); > >> break; > >> case L2CAP_CONF_EWS: > >> + if (olen != 2) > >> + break; > >> txwin_ext = val; > >> break; > >> } > >> -- > >> 2.17.1 > >> > >> > >> -- > >> kernel-team mailing list > >> kernel-team@lists.ubuntu.com > >> https://lists.ubuntu.com/mailman/listinfo/kernel-team >
On 2019-02-19 20:27:46, Kai-Heng Feng wrote: > From: Marcel Holtmann <marcel@holtmann.org> > > When doing option parsing for standard type values of 1, 2 or 4 octets, > the value is converted directly into a variable instead of a pointer. To > avoid being tricked into being a pointer, check that for these option > types that sizes actually match. In L2CAP every option is fixed size and > thus it is prudent anyway to ensure that the remote side sends us the > right option size along with option paramters. > > If the option size is not matching the option type, then that option is > silently ignored. It is a protocol violation and instead of trying to > give the remote attacker any further hints just pretend that option is > not present and proceed with the default values. Implementation > following the specification and its qualification procedures will always > use the correct size and thus not being impacted here. > > To keep the code readable and consistent accross all options, a few > cosmetic changes were also required. > > CVE-2019-3460 > > Signed-off-by: Marcel Holtmann <marcel@holtmann.org> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> > (backported from commit af3d5d1c87664a4f150fcf3534c6567cb19909b0 linux-next) > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Only contextual changes were required for the backport and the changes look straightforward. Acked-by: Tyler Hicks <tyhicks@canonical.com> Tyler > --- > net/bluetooth/l2cap_core.c | 77 +++++++++++++++++++++++--------------- > 1 file changed, 46 insertions(+), 31 deletions(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 61d0f290c0c6..a7de6cbdeced 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -3284,10 +3284,14 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > > switch (type) { > case L2CAP_CONF_MTU: > + if (olen != 2) > + break; > mtu = val; > break; > > case L2CAP_CONF_FLUSH_TO: > + if (olen != 2) > + break; > chan->flush_to = val; > break; > > @@ -3295,26 +3299,30 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > break; > > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *) val, olen); > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *) val, olen); > break; > > case L2CAP_CONF_FCS: > + if (olen != 1) > + break; > if (val == L2CAP_FCS_NONE) > set_bit(CONF_RECV_NO_FCS, &chan->conf_state); > break; > > case L2CAP_CONF_EFS: > - if (olen == sizeof(efs)) { > - remote_efs = 1; > - memcpy(&efs, (void *) val, olen); > - } > + if (olen != sizeof(efs)) > + break; > + remote_efs = 1; > + memcpy(&efs, (void *) val, olen); > break; > > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > if (!chan->conn->hs_enabled) > return -ECONNREFUSED; > - > set_bit(FLAG_EXT_CTRL, &chan->flags); > set_bit(CONF_EWS_RECV, &chan->conf_state); > chan->tx_win_max = L2CAP_DEFAULT_EXT_WINDOW; > @@ -3324,7 +3332,6 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > default: > if (hint) > break; > - > result = L2CAP_CONF_UNKNOWN; > *((u8 *) ptr++) = type; > break; > @@ -3492,55 +3499,60 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len, > > switch (type) { > case L2CAP_CONF_MTU: > + if (olen != 2) > + break; > if (val < L2CAP_DEFAULT_MIN_MTU) { > *result = L2CAP_CONF_UNACCEPT; > chan->imtu = L2CAP_DEFAULT_MIN_MTU; > } else > chan->imtu = val; > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, > + endptr - ptr); > break; > > case L2CAP_CONF_FLUSH_TO: > + if (olen != 2) > + break; > chan->flush_to = val; > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, > - 2, chan->flush_to, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, > + chan->flush_to, endptr - ptr); > break; > > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *)val, olen); > - > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *)val, olen); > if (test_bit(CONF_STATE2_DEVICE, &chan->conf_state) && > rfc.mode != chan->mode) > return -ECONNREFUSED; > - > chan->fcs = 0; > - > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, > - sizeof(rfc), (unsigned long) &rfc, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc), > + (unsigned long) &rfc, endptr - ptr); > break; > > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > chan->ack_win = min_t(u16, val, chan->ack_win); > l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 2, > chan->tx_win, endptr - ptr); > break; > > case L2CAP_CONF_EFS: > - if (olen == sizeof(efs)) { > - memcpy(&efs, (void *)val, olen); > - > - if (chan->local_stype != L2CAP_SERV_NOTRAFIC && > - efs.stype != L2CAP_SERV_NOTRAFIC && > - efs.stype != chan->local_stype) > - return -ECONNREFUSED; > - > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), > - (unsigned long) &efs, endptr - ptr); > - } > + if (olen != sizeof(efs)) > + break; > + memcpy(&efs, (void *)val, olen); > + if (chan->local_stype != L2CAP_SERV_NOTRAFIC && > + efs.stype != L2CAP_SERV_NOTRAFIC && > + efs.stype != chan->local_stype) > + return -ECONNREFUSED; > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), > + (unsigned long) &efs, endptr - ptr); > break; > > case L2CAP_CONF_FCS: > + if (olen != 1) > + break; > if (*result == L2CAP_CONF_PENDING) > if (val == L2CAP_FCS_NONE) > set_bit(CONF_RECV_NO_FCS, > @@ -3655,10 +3667,13 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len) > > switch (type) { > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *)val, olen); > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *)val, olen); > break; > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > txwin_ext = val; > break; > } > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 2/19/19 1:27 PM, Kai-Heng Feng wrote: > From: Marcel Holtmann <marcel@holtmann.org> > > When doing option parsing for standard type values of 1, 2 or 4 octets, > the value is converted directly into a variable instead of a pointer. To > avoid being tricked into being a pointer, check that for these option > types that sizes actually match. In L2CAP every option is fixed size and > thus it is prudent anyway to ensure that the remote side sends us the > right option size along with option paramters. > > If the option size is not matching the option type, then that option is > silently ignored. It is a protocol violation and instead of trying to > give the remote attacker any further hints just pretend that option is > not present and proceed with the default values. Implementation > following the specification and its qualification procedures will always > use the correct size and thus not being impacted here. > > To keep the code readable and consistent accross all options, a few > cosmetic changes were also required. > > CVE-2019-3460 > > Signed-off-by: Marcel Holtmann <marcel@holtmann.org> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> > (backported from commit af3d5d1c87664a4f150fcf3534c6567cb19909b0 linux-next) > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> The commit is already merged into Linus' tree, so the "linux-next" part can be removed from the provenance line. Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > net/bluetooth/l2cap_core.c | 77 +++++++++++++++++++++++--------------- > 1 file changed, 46 insertions(+), 31 deletions(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 61d0f290c0c6..a7de6cbdeced 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -3284,10 +3284,14 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > > switch (type) { > case L2CAP_CONF_MTU: > + if (olen != 2) > + break; > mtu = val; > break; > > case L2CAP_CONF_FLUSH_TO: > + if (olen != 2) > + break; > chan->flush_to = val; > break; > > @@ -3295,26 +3299,30 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > break; > > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *) val, olen); > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *) val, olen); > break; > > case L2CAP_CONF_FCS: > + if (olen != 1) > + break; > if (val == L2CAP_FCS_NONE) > set_bit(CONF_RECV_NO_FCS, &chan->conf_state); > break; > > case L2CAP_CONF_EFS: > - if (olen == sizeof(efs)) { > - remote_efs = 1; > - memcpy(&efs, (void *) val, olen); > - } > + if (olen != sizeof(efs)) > + break; > + remote_efs = 1; > + memcpy(&efs, (void *) val, olen); > break; > > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > if (!chan->conn->hs_enabled) > return -ECONNREFUSED; > - > set_bit(FLAG_EXT_CTRL, &chan->flags); > set_bit(CONF_EWS_RECV, &chan->conf_state); > chan->tx_win_max = L2CAP_DEFAULT_EXT_WINDOW; > @@ -3324,7 +3332,6 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > default: > if (hint) > break; > - > result = L2CAP_CONF_UNKNOWN; > *((u8 *) ptr++) = type; > break; > @@ -3492,55 +3499,60 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len, > > switch (type) { > case L2CAP_CONF_MTU: > + if (olen != 2) > + break; > if (val < L2CAP_DEFAULT_MIN_MTU) { > *result = L2CAP_CONF_UNACCEPT; > chan->imtu = L2CAP_DEFAULT_MIN_MTU; > } else > chan->imtu = val; > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, > + endptr - ptr); > break; > > case L2CAP_CONF_FLUSH_TO: > + if (olen != 2) > + break; > chan->flush_to = val; > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, > - 2, chan->flush_to, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, > + chan->flush_to, endptr - ptr); > break; > > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *)val, olen); > - > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *)val, olen); > if (test_bit(CONF_STATE2_DEVICE, &chan->conf_state) && > rfc.mode != chan->mode) > return -ECONNREFUSED; > - > chan->fcs = 0; > - > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, > - sizeof(rfc), (unsigned long) &rfc, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc), > + (unsigned long) &rfc, endptr - ptr); > break; > > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > chan->ack_win = min_t(u16, val, chan->ack_win); > l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 2, > chan->tx_win, endptr - ptr); > break; > > case L2CAP_CONF_EFS: > - if (olen == sizeof(efs)) { > - memcpy(&efs, (void *)val, olen); > - > - if (chan->local_stype != L2CAP_SERV_NOTRAFIC && > - efs.stype != L2CAP_SERV_NOTRAFIC && > - efs.stype != chan->local_stype) > - return -ECONNREFUSED; > - > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), > - (unsigned long) &efs, endptr - ptr); > - } > + if (olen != sizeof(efs)) > + break; > + memcpy(&efs, (void *)val, olen); > + if (chan->local_stype != L2CAP_SERV_NOTRAFIC && > + efs.stype != L2CAP_SERV_NOTRAFIC && > + efs.stype != chan->local_stype) > + return -ECONNREFUSED; > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), > + (unsigned long) &efs, endptr - ptr); > break; > > case L2CAP_CONF_FCS: > + if (olen != 1) > + break; > if (*result == L2CAP_CONF_PENDING) > if (val == L2CAP_FCS_NONE) > set_bit(CONF_RECV_NO_FCS, > @@ -3655,10 +3667,13 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len) > > switch (type) { > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *)val, olen); > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *)val, olen); > break; > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > txwin_ext = val; > break; > }
On 2/19/19 1:27 PM, Kai-Heng Feng wrote: > From: Marcel Holtmann <marcel@holtmann.org> > > When doing option parsing for standard type values of 1, 2 or 4 octets, > the value is converted directly into a variable instead of a pointer. To > avoid being tricked into being a pointer, check that for these option > types that sizes actually match. In L2CAP every option is fixed size and > thus it is prudent anyway to ensure that the remote side sends us the > right option size along with option paramters. > > If the option size is not matching the option type, then that option is > silently ignored. It is a protocol violation and instead of trying to > give the remote attacker any further hints just pretend that option is > not present and proceed with the default values. Implementation > following the specification and its qualification procedures will always > use the correct size and thus not being impacted here. > > To keep the code readable and consistent accross all options, a few > cosmetic changes were also required. > > CVE-2019-3460 > > Signed-off-by: Marcel Holtmann <marcel@holtmann.org> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> > (backported from commit af3d5d1c87664a4f150fcf3534c6567cb19909b0 linux-next) > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Applied to trusty/master-next branch, removing "linux-next" from the provenance live above. Thanks, Kleber > --- > net/bluetooth/l2cap_core.c | 77 +++++++++++++++++++++++--------------- > 1 file changed, 46 insertions(+), 31 deletions(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 61d0f290c0c6..a7de6cbdeced 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -3284,10 +3284,14 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > > switch (type) { > case L2CAP_CONF_MTU: > + if (olen != 2) > + break; > mtu = val; > break; > > case L2CAP_CONF_FLUSH_TO: > + if (olen != 2) > + break; > chan->flush_to = val; > break; > > @@ -3295,26 +3299,30 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > break; > > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *) val, olen); > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *) val, olen); > break; > > case L2CAP_CONF_FCS: > + if (olen != 1) > + break; > if (val == L2CAP_FCS_NONE) > set_bit(CONF_RECV_NO_FCS, &chan->conf_state); > break; > > case L2CAP_CONF_EFS: > - if (olen == sizeof(efs)) { > - remote_efs = 1; > - memcpy(&efs, (void *) val, olen); > - } > + if (olen != sizeof(efs)) > + break; > + remote_efs = 1; > + memcpy(&efs, (void *) val, olen); > break; > > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > if (!chan->conn->hs_enabled) > return -ECONNREFUSED; > - > set_bit(FLAG_EXT_CTRL, &chan->flags); > set_bit(CONF_EWS_RECV, &chan->conf_state); > chan->tx_win_max = L2CAP_DEFAULT_EXT_WINDOW; > @@ -3324,7 +3332,6 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > default: > if (hint) > break; > - > result = L2CAP_CONF_UNKNOWN; > *((u8 *) ptr++) = type; > break; > @@ -3492,55 +3499,60 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len, > > switch (type) { > case L2CAP_CONF_MTU: > + if (olen != 2) > + break; > if (val < L2CAP_DEFAULT_MIN_MTU) { > *result = L2CAP_CONF_UNACCEPT; > chan->imtu = L2CAP_DEFAULT_MIN_MTU; > } else > chan->imtu = val; > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, > + endptr - ptr); > break; > > case L2CAP_CONF_FLUSH_TO: > + if (olen != 2) > + break; > chan->flush_to = val; > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, > - 2, chan->flush_to, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, > + chan->flush_to, endptr - ptr); > break; > > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *)val, olen); > - > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *)val, olen); > if (test_bit(CONF_STATE2_DEVICE, &chan->conf_state) && > rfc.mode != chan->mode) > return -ECONNREFUSED; > - > chan->fcs = 0; > - > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, > - sizeof(rfc), (unsigned long) &rfc, endptr - ptr); > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc), > + (unsigned long) &rfc, endptr - ptr); > break; > > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > chan->ack_win = min_t(u16, val, chan->ack_win); > l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 2, > chan->tx_win, endptr - ptr); > break; > > case L2CAP_CONF_EFS: > - if (olen == sizeof(efs)) { > - memcpy(&efs, (void *)val, olen); > - > - if (chan->local_stype != L2CAP_SERV_NOTRAFIC && > - efs.stype != L2CAP_SERV_NOTRAFIC && > - efs.stype != chan->local_stype) > - return -ECONNREFUSED; > - > - l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), > - (unsigned long) &efs, endptr - ptr); > - } > + if (olen != sizeof(efs)) > + break; > + memcpy(&efs, (void *)val, olen); > + if (chan->local_stype != L2CAP_SERV_NOTRAFIC && > + efs.stype != L2CAP_SERV_NOTRAFIC && > + efs.stype != chan->local_stype) > + return -ECONNREFUSED; > + l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), > + (unsigned long) &efs, endptr - ptr); > break; > > case L2CAP_CONF_FCS: > + if (olen != 1) > + break; > if (*result == L2CAP_CONF_PENDING) > if (val == L2CAP_FCS_NONE) > set_bit(CONF_RECV_NO_FCS, > @@ -3655,10 +3667,13 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len) > > switch (type) { > case L2CAP_CONF_RFC: > - if (olen == sizeof(rfc)) > - memcpy(&rfc, (void *)val, olen); > + if (olen != sizeof(rfc)) > + break; > + memcpy(&rfc, (void *)val, olen); > break; > case L2CAP_CONF_EWS: > + if (olen != 2) > + break; > txwin_ext = val; > break; > }
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 61d0f290c0c6..a7de6cbdeced 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -3284,10 +3284,14 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data switch (type) { case L2CAP_CONF_MTU: + if (olen != 2) + break; mtu = val; break; case L2CAP_CONF_FLUSH_TO: + if (olen != 2) + break; chan->flush_to = val; break; @@ -3295,26 +3299,30 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data break; case L2CAP_CONF_RFC: - if (olen == sizeof(rfc)) - memcpy(&rfc, (void *) val, olen); + if (olen != sizeof(rfc)) + break; + memcpy(&rfc, (void *) val, olen); break; case L2CAP_CONF_FCS: + if (olen != 1) + break; if (val == L2CAP_FCS_NONE) set_bit(CONF_RECV_NO_FCS, &chan->conf_state); break; case L2CAP_CONF_EFS: - if (olen == sizeof(efs)) { - remote_efs = 1; - memcpy(&efs, (void *) val, olen); - } + if (olen != sizeof(efs)) + break; + remote_efs = 1; + memcpy(&efs, (void *) val, olen); break; case L2CAP_CONF_EWS: + if (olen != 2) + break; if (!chan->conn->hs_enabled) return -ECONNREFUSED; - set_bit(FLAG_EXT_CTRL, &chan->flags); set_bit(CONF_EWS_RECV, &chan->conf_state); chan->tx_win_max = L2CAP_DEFAULT_EXT_WINDOW; @@ -3324,7 +3332,6 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data default: if (hint) break; - result = L2CAP_CONF_UNKNOWN; *((u8 *) ptr++) = type; break; @@ -3492,55 +3499,60 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len, switch (type) { case L2CAP_CONF_MTU: + if (olen != 2) + break; if (val < L2CAP_DEFAULT_MIN_MTU) { *result = L2CAP_CONF_UNACCEPT; chan->imtu = L2CAP_DEFAULT_MIN_MTU; } else chan->imtu = val; - l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, endptr - ptr); + l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, + endptr - ptr); break; case L2CAP_CONF_FLUSH_TO: + if (olen != 2) + break; chan->flush_to = val; - l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, - 2, chan->flush_to, endptr - ptr); + l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, + chan->flush_to, endptr - ptr); break; case L2CAP_CONF_RFC: - if (olen == sizeof(rfc)) - memcpy(&rfc, (void *)val, olen); - + if (olen != sizeof(rfc)) + break; + memcpy(&rfc, (void *)val, olen); if (test_bit(CONF_STATE2_DEVICE, &chan->conf_state) && rfc.mode != chan->mode) return -ECONNREFUSED; - chan->fcs = 0; - - l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, - sizeof(rfc), (unsigned long) &rfc, endptr - ptr); + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc), + (unsigned long) &rfc, endptr - ptr); break; case L2CAP_CONF_EWS: + if (olen != 2) + break; chan->ack_win = min_t(u16, val, chan->ack_win); l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 2, chan->tx_win, endptr - ptr); break; case L2CAP_CONF_EFS: - if (olen == sizeof(efs)) { - memcpy(&efs, (void *)val, olen); - - if (chan->local_stype != L2CAP_SERV_NOTRAFIC && - efs.stype != L2CAP_SERV_NOTRAFIC && - efs.stype != chan->local_stype) - return -ECONNREFUSED; - - l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), - (unsigned long) &efs, endptr - ptr); - } + if (olen != sizeof(efs)) + break; + memcpy(&efs, (void *)val, olen); + if (chan->local_stype != L2CAP_SERV_NOTRAFIC && + efs.stype != L2CAP_SERV_NOTRAFIC && + efs.stype != chan->local_stype) + return -ECONNREFUSED; + l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), + (unsigned long) &efs, endptr - ptr); break; case L2CAP_CONF_FCS: + if (olen != 1) + break; if (*result == L2CAP_CONF_PENDING) if (val == L2CAP_FCS_NONE) set_bit(CONF_RECV_NO_FCS, @@ -3655,10 +3667,13 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len) switch (type) { case L2CAP_CONF_RFC: - if (olen == sizeof(rfc)) - memcpy(&rfc, (void *)val, olen); + if (olen != sizeof(rfc)) + break; + memcpy(&rfc, (void *)val, olen); break; case L2CAP_CONF_EWS: + if (olen != 2) + break; txwin_ext = val; break; }