Skip to content

Commit d7ee5bd

Browse files
committed
Merge tag 'firewire-fixes-6.17-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394
Pull firewire fixes from Takashi Sakamoto: "This fixes a potential call to schedule() within an RCU read-side critical section. The solution applies reference counting to ensure that handlers which may call schedule() are invoked safely outside of the critical section" * tag 'firewire-fixes-6.17-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394: firewire: core: reallocate buffer for FCP address handlers when more than 4 are registered firewire: core: call FCP address handlers outside RCU read-side critical section firewire: core: call handler for exclusive regions outside RCU read-side critical section firewire: core: use reference counting to invoke address handlers safely
2 parents 24ea63e + 0342273 commit d7ee5bd

File tree

2 files changed

+85
-10
lines changed

2 files changed

+85
-10
lines changed

drivers/firewire/core-transaction.c

Lines changed: 81 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,23 @@ const struct fw_address_region fw_unit_space_region =
550550
{ .start = 0xfffff0000900ULL, .end = 0x1000000000000ULL, };
551551
#endif /* 0 */
552552

553+
static void complete_address_handler(struct kref *kref)
554+
{
555+
struct fw_address_handler *handler = container_of(kref, struct fw_address_handler, kref);
556+
557+
complete(&handler->done);
558+
}
559+
560+
static void get_address_handler(struct fw_address_handler *handler)
561+
{
562+
kref_get(&handler->kref);
563+
}
564+
565+
static int put_address_handler(struct fw_address_handler *handler)
566+
{
567+
return kref_put(&handler->kref, complete_address_handler);
568+
}
569+
553570
/**
554571
* fw_core_add_address_handler() - register for incoming requests
555572
* @handler: callback
@@ -596,6 +613,8 @@ int fw_core_add_address_handler(struct fw_address_handler *handler,
596613
if (other != NULL) {
597614
handler->offset += other->length;
598615
} else {
616+
init_completion(&handler->done);
617+
kref_init(&handler->kref);
599618
list_add_tail_rcu(&handler->link, &address_handler_list);
600619
ret = 0;
601620
break;
@@ -621,6 +640,9 @@ void fw_core_remove_address_handler(struct fw_address_handler *handler)
621640
list_del_rcu(&handler->link);
622641

623642
synchronize_rcu();
643+
644+
if (!put_address_handler(handler))
645+
wait_for_completion(&handler->done);
624646
}
625647
EXPORT_SYMBOL(fw_core_remove_address_handler);
626648

@@ -914,22 +936,31 @@ static void handle_exclusive_region_request(struct fw_card *card,
914936
handler = lookup_enclosing_address_handler(&address_handler_list, offset,
915937
request->length);
916938
if (handler)
917-
handler->address_callback(card, request, tcode, destination, source,
918-
p->generation, offset, request->data,
919-
request->length, handler->callback_data);
939+
get_address_handler(handler);
920940
}
921941

922-
if (!handler)
942+
if (!handler) {
923943
fw_send_response(card, request, RCODE_ADDRESS_ERROR);
944+
return;
945+
}
946+
947+
// Outside the RCU read-side critical section. Without spinlock. With reference count.
948+
handler->address_callback(card, request, tcode, destination, source, p->generation, offset,
949+
request->data, request->length, handler->callback_data);
950+
put_address_handler(handler);
924951
}
925952

953+
// To use kmalloc allocator efficiently, this should be power of two.
954+
#define BUFFER_ON_KERNEL_STACK_SIZE 4
955+
926956
static void handle_fcp_region_request(struct fw_card *card,
927957
struct fw_packet *p,
928958
struct fw_request *request,
929959
unsigned long long offset)
930960
{
931-
struct fw_address_handler *handler;
932-
int tcode, destination, source;
961+
struct fw_address_handler *buffer_on_kernel_stack[BUFFER_ON_KERNEL_STACK_SIZE];
962+
struct fw_address_handler *handler, **handlers;
963+
int tcode, destination, source, i, count, buffer_size;
933964

934965
if ((offset != (CSR_REGISTER_BASE | CSR_FCP_COMMAND) &&
935966
offset != (CSR_REGISTER_BASE | CSR_FCP_RESPONSE)) ||
@@ -950,15 +981,55 @@ static void handle_fcp_region_request(struct fw_card *card,
950981
return;
951982
}
952983

984+
count = 0;
985+
handlers = buffer_on_kernel_stack;
986+
buffer_size = ARRAY_SIZE(buffer_on_kernel_stack);
953987
scoped_guard(rcu) {
954988
list_for_each_entry_rcu(handler, &address_handler_list, link) {
955-
if (is_enclosing_handler(handler, offset, request->length))
956-
handler->address_callback(card, request, tcode, destination, source,
957-
p->generation, offset, request->data,
958-
request->length, handler->callback_data);
989+
if (is_enclosing_handler(handler, offset, request->length)) {
990+
if (count >= buffer_size) {
991+
int next_size = buffer_size * 2;
992+
struct fw_address_handler **buffer_on_kernel_heap;
993+
994+
if (handlers == buffer_on_kernel_stack)
995+
buffer_on_kernel_heap = NULL;
996+
else
997+
buffer_on_kernel_heap = handlers;
998+
999+
buffer_on_kernel_heap =
1000+
krealloc_array(buffer_on_kernel_heap, next_size,
1001+
sizeof(*buffer_on_kernel_heap), GFP_ATOMIC);
1002+
// FCP is used for purposes unrelated to significant system
1003+
// resources (e.g. storage or networking), so allocation
1004+
// failures are not considered so critical.
1005+
if (!buffer_on_kernel_heap)
1006+
break;
1007+
1008+
if (handlers == buffer_on_kernel_stack) {
1009+
memcpy(buffer_on_kernel_heap, buffer_on_kernel_stack,
1010+
sizeof(buffer_on_kernel_stack));
1011+
}
1012+
1013+
handlers = buffer_on_kernel_heap;
1014+
buffer_size = next_size;
1015+
}
1016+
get_address_handler(handler);
1017+
handlers[count++] = handler;
1018+
}
9591019
}
9601020
}
9611021

1022+
for (i = 0; i < count; ++i) {
1023+
handler = handlers[i];
1024+
handler->address_callback(card, request, tcode, destination, source,
1025+
p->generation, offset, request->data,
1026+
request->length, handler->callback_data);
1027+
put_address_handler(handler);
1028+
}
1029+
1030+
if (handlers != buffer_on_kernel_stack)
1031+
kfree(handlers);
1032+
9621033
fw_send_response(card, request, RCODE_COMPLETE);
9631034
}
9641035

include/linux/firewire.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,11 @@ struct fw_address_handler {
341341
u64 length;
342342
fw_address_callback_t address_callback;
343343
void *callback_data;
344+
345+
// Only for core functions.
344346
struct list_head link;
347+
struct kref kref;
348+
struct completion done;
345349
};
346350

347351
struct fw_address_region {

0 commit comments

Comments
 (0)