Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

isdn/gigaset: fix non-heap pointer deallocation

at_state structures may be allocated individually or as part of a
cardstate or bc_state structure. The disconnect() function handled
both cases, creating a risk that it might try to deallocate an
at_state structure that had not been allocated individually.
Fix by splitting disconnect() into two variants handling cases
with and without an associated B channel separately, and adding
an explicit check.

Spotted with Coverity.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Tilman Schmidt and committed by
David S. Miller
b8324f94 846ac301

+70 -41
+70 -41
drivers/isdn/gigaset/ev-layer.c
··· 604 604 } 605 605 EXPORT_SYMBOL_GPL(gigaset_handle_modem_response); 606 606 607 - /* disconnect 607 + /* disconnect_nobc 608 608 * process closing of connection associated with given AT state structure 609 + * without B channel 609 610 */ 610 - static void disconnect(struct at_state_t **at_state_p) 611 + static void disconnect_nobc(struct at_state_t **at_state_p, 612 + struct cardstate *cs) 611 613 { 612 614 unsigned long flags; 613 - struct bc_state *bcs = (*at_state_p)->bcs; 614 - struct cardstate *cs = (*at_state_p)->cs; 615 615 616 616 spin_lock_irqsave(&cs->lock, flags); 617 617 ++(*at_state_p)->seq_index; ··· 622 622 gig_dbg(DEBUG_EVENT, "Scheduling PC_UMMODE"); 623 623 cs->commands_pending = 1; 624 624 } 625 - spin_unlock_irqrestore(&cs->lock, flags); 626 625 627 - if (bcs) { 628 - /* B channel assigned: invoke hardware specific handler */ 629 - cs->ops->close_bchannel(bcs); 630 - /* notify LL */ 631 - if (bcs->chstate & (CHS_D_UP | CHS_NOTIFY_LL)) { 632 - bcs->chstate &= ~(CHS_D_UP | CHS_NOTIFY_LL); 633 - gigaset_isdn_hupD(bcs); 634 - } 635 - } else { 636 - /* no B channel assigned: just deallocate */ 637 - spin_lock_irqsave(&cs->lock, flags); 626 + /* check for and deallocate temporary AT state */ 627 + if (!list_empty(&(*at_state_p)->list)) { 638 628 list_del(&(*at_state_p)->list); 639 629 kfree(*at_state_p); 640 630 *at_state_p = NULL; 641 - spin_unlock_irqrestore(&cs->lock, flags); 631 + } 632 + 633 + spin_unlock_irqrestore(&cs->lock, flags); 634 + } 635 + 636 + /* disconnect_bc 637 + * process closing of connection associated with given AT state structure 638 + * and B channel 639 + */ 640 + static void disconnect_bc(struct at_state_t *at_state, 641 + struct cardstate *cs, struct bc_state *bcs) 642 + { 643 + unsigned long flags; 644 + 645 + spin_lock_irqsave(&cs->lock, flags); 646 + ++at_state->seq_index; 647 + 648 + /* revert to selected idle mode */ 649 + if (!cs->cidmode) { 650 + cs->at_state.pending_commands |= PC_UMMODE; 651 + gig_dbg(DEBUG_EVENT, "Scheduling PC_UMMODE"); 652 + cs->commands_pending = 1; 653 + } 654 + spin_unlock_irqrestore(&cs->lock, flags); 655 + 656 + /* invoke hardware specific handler */ 657 + cs->ops->close_bchannel(bcs); 658 + 659 + /* notify LL */ 660 + if (bcs->chstate & (CHS_D_UP | CHS_NOTIFY_LL)) { 661 + bcs->chstate &= ~(CHS_D_UP | CHS_NOTIFY_LL); 662 + gigaset_isdn_hupD(bcs); 642 663 } 643 664 } 644 665 ··· 667 646 * get a free AT state structure: either one of those associated with the 668 647 * B channels of the Gigaset device, or if none of those is available, 669 648 * a newly allocated one with bcs=NULL 670 - * The structure should be freed by calling disconnect() after use. 649 + * The structure should be freed by calling disconnect_nobc() after use. 671 650 */ 672 651 static inline struct at_state_t *get_free_channel(struct cardstate *cs, 673 652 int cid) ··· 1078 1057 struct event_t *ev) 1079 1058 { 1080 1059 struct at_state_t *at_state = *p_at_state; 1081 - struct at_state_t *at_state2; 1060 + struct bc_state *bcs2; 1082 1061 unsigned long flags; 1083 1062 1084 1063 int channel; ··· 1177 1156 break; 1178 1157 case ACT_RING: 1179 1158 /* get fresh AT state structure for new CID */ 1180 - at_state2 = get_free_channel(cs, ev->parameter); 1181 - if (!at_state2) { 1159 + at_state = get_free_channel(cs, ev->parameter); 1160 + if (!at_state) { 1182 1161 dev_warn(cs->dev, 1183 1162 "RING ignored: could not allocate channel structure\n"); 1184 1163 break; ··· 1187 1166 /* initialize AT state structure 1188 1167 * note that bcs may be NULL if no B channel is free 1189 1168 */ 1190 - at_state2->ConState = 700; 1169 + at_state->ConState = 700; 1191 1170 for (i = 0; i < STR_NUM; ++i) { 1192 - kfree(at_state2->str_var[i]); 1193 - at_state2->str_var[i] = NULL; 1171 + kfree(at_state->str_var[i]); 1172 + at_state->str_var[i] = NULL; 1194 1173 } 1195 - at_state2->int_var[VAR_ZCTP] = -1; 1174 + at_state->int_var[VAR_ZCTP] = -1; 1196 1175 1197 1176 spin_lock_irqsave(&cs->lock, flags); 1198 - at_state2->timer_expires = RING_TIMEOUT; 1199 - at_state2->timer_active = 1; 1177 + at_state->timer_expires = RING_TIMEOUT; 1178 + at_state->timer_active = 1; 1200 1179 spin_unlock_irqrestore(&cs->lock, flags); 1201 1180 break; 1202 1181 case ACT_ICALL: ··· 1234 1213 case ACT_DISCONNECT: 1235 1214 cs->cur_at_seq = SEQ_NONE; 1236 1215 at_state->cid = -1; 1237 - if (bcs && cs->onechannel && cs->dle) { 1216 + if (!bcs) { 1217 + disconnect_nobc(p_at_state, cs); 1218 + } else if (cs->onechannel && cs->dle) { 1238 1219 /* Check for other open channels not needed: 1239 1220 * DLE only used for M10x with one B channel. 1240 1221 */ 1241 1222 at_state->pending_commands |= PC_DLE0; 1242 1223 cs->commands_pending = 1; 1243 - } else 1244 - disconnect(p_at_state); 1224 + } else { 1225 + disconnect_bc(at_state, cs, bcs); 1226 + } 1245 1227 break; 1246 1228 case ACT_FAKEDLE0: 1247 1229 at_state->int_var[VAR_ZDLE] = 0; ··· 1252 1228 /* fall through */ 1253 1229 case ACT_DLE0: 1254 1230 cs->cur_at_seq = SEQ_NONE; 1255 - at_state2 = &cs->bcs[cs->curchannel].at_state; 1256 - disconnect(&at_state2); 1231 + bcs2 = cs->bcs + cs->curchannel; 1232 + disconnect_bc(&bcs2->at_state, cs, bcs2); 1257 1233 break; 1258 1234 case ACT_ABORTHUP: 1259 1235 cs->cur_at_seq = SEQ_NONE; 1260 1236 dev_warn(cs->dev, "Could not hang up.\n"); 1261 1237 at_state->cid = -1; 1262 - if (bcs && cs->onechannel) 1238 + if (!bcs) 1239 + disconnect_nobc(p_at_state, cs); 1240 + else if (cs->onechannel) 1263 1241 at_state->pending_commands |= PC_DLE0; 1264 1242 else 1265 - disconnect(p_at_state); 1243 + disconnect_bc(at_state, cs, bcs); 1266 1244 schedule_init(cs, MS_RECOVER); 1267 1245 break; 1268 1246 case ACT_FAILDLE0: 1269 1247 cs->cur_at_seq = SEQ_NONE; 1270 1248 dev_warn(cs->dev, "Error leaving DLE mode.\n"); 1271 1249 cs->dle = 0; 1272 - at_state2 = &cs->bcs[cs->curchannel].at_state; 1273 - disconnect(&at_state2); 1250 + bcs2 = cs->bcs + cs->curchannel; 1251 + disconnect_bc(&bcs2->at_state, cs, bcs2); 1274 1252 schedule_init(cs, MS_RECOVER); 1275 1253 break; 1276 1254 case ACT_FAILDLE1: ··· 1301 1275 if (reinit_and_retry(cs, channel) < 0) { 1302 1276 dev_warn(cs->dev, 1303 1277 "Could not get a call ID. Cannot dial.\n"); 1304 - at_state2 = &cs->bcs[channel].at_state; 1305 - disconnect(&at_state2); 1278 + bcs2 = cs->bcs + channel; 1279 + disconnect_bc(&bcs2->at_state, cs, bcs2); 1306 1280 } 1307 1281 break; 1308 1282 case ACT_ABORTCID: 1309 1283 cs->cur_at_seq = SEQ_NONE; 1310 - at_state2 = &cs->bcs[cs->curchannel].at_state; 1311 - disconnect(&at_state2); 1284 + bcs2 = cs->bcs + cs->curchannel; 1285 + disconnect_bc(&bcs2->at_state, cs, bcs2); 1312 1286 break; 1313 1287 1314 1288 case ACT_DIALING: ··· 1317 1291 break; 1318 1292 1319 1293 case ACT_ABORTACCEPT: /* hangup/error/timeout during ICALL procssng */ 1320 - disconnect(p_at_state); 1294 + if (bcs) 1295 + disconnect_bc(at_state, cs, bcs); 1296 + else 1297 + disconnect_nobc(p_at_state, cs); 1321 1298 break; 1322 1299 1323 1300 case ACT_ABORTDIAL: /* error/timeout during dial preparation */